-
Notifications
You must be signed in to change notification settings - Fork 45
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
Dedent multiline text to preserve content indentation #197
Conversation
@Pike This is based on your branch from #162 (comment). Here's a summary of differences:
|
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.
Just two comment nits, r=me.
syntax/abstract.mjs
Outdated
@@ -154,18 +159,36 @@ function attach_comments(acc, cur) { | |||
} | |||
} | |||
|
|||
const LEADING_BLANK = /^[ \n\r]*/; | |||
const TRAILING_BLANK = /[ \n\r]*$/; | |||
// Remove the largest common indentation from a list of PatternElements. |
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.
Extend this comment to make it expressive that all literal strings are indents, thanks to grammar.mjs separating those out?
function join_adjacent(...types) { | ||
return function(acc, cur) { | ||
let prev = acc[acc.length - 1]; | ||
for (let Type of types) { | ||
if (prev instanceof Type && cur instanceof Type) { | ||
join_of_type(Type, prev, cur); | ||
// Replace prev with a new node of the same type whose value is | ||
// the sum of prev and cur, and discard cur. |
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.
... discard cur and prev.
?
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.
Do you think it's needed given that the comment starts with Replace...
?
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 could go without the , and discard ...
altogether, too.
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.
Not particularly opinionated on that last comment, r=me.
function join_adjacent(...types) { | ||
return function(acc, cur) { | ||
let prev = acc[acc.length - 1]; | ||
for (let Type of types) { | ||
if (prev instanceof Type && cur instanceof Type) { | ||
join_of_type(Type, prev, cur); | ||
// Replace prev with a new node of the same type whose value is | ||
// the sum of prev and cur, and discard cur. |
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 could go without the , and discard ...
altogether, too.
Fix #162.