-
Notifications
You must be signed in to change notification settings - Fork 5
fix: fix dedenting feature #90
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
|
||
| config.enable_source_link = | ||
| extracted?.enable_source_link ?? true | ||
| ? current.enable_source_link |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is fancy looking but I thought it was too hard to read
| for (const file of files) { | ||
| await writeAsync( | ||
| file.fullpath, | ||
| file.fileString(this.config.features.enable_code_dedenting) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I removed the old dedenting implementation to avoid confusing.
| if (config.select !== undefined) { | ||
| const selectedLines = selectLines(config.select, this.lines, this.ext); | ||
| lines.push(...selectedLines); | ||
| let snippetLines = selectLines(config.select, this.lines, this.ext); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is where we assemble the lines of the snippet and where I run the lines through the dedenting functions
axfelix
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excellent and thorough changes!
Previous implementation of de-indenting was flawed. It would de-indent every line in the whole Markdown file if turned on - including lists and other text that we don't want to de-indent. This was not caught in the test cases since the entire test case only had a code snippet but no preceding or succeeding lines.
See the results in the diffs of this PR: https://github.com/temporalio/documentation/pull/3840/files