Adding user-defined group delimiters #104
Conversation
Yeah, I hadn't fully considered how we split into lines earlier in the flow than we parse options for a block. That certainly complicates that suggestion a bit. I wonder how bad it would be to pass the full file into
Could you also add some goldens that capture that subtlety? Especially cases where one of the group_delimiter_regexes matches the sticky_prefixed line |
| // StickyPrefixes tells us about other types of lines that should behave as sticky comments. | ||
| StickyPrefixes map[string]bool `key:"sticky_prefixes"` | ||
| // GroupDelimiterRegexes tells us if a line is allowed to end a group. | ||
| GroupDelimiterRegexes []RegexOption `key:"group_delimiter_regexes"` |
There was a problem hiding this comment.
What happens if someone provides a replacement template with this option? Is that even valid?
I wonder if it should actually be separate from the ByRegexpOption
| }, | ||
| }, | ||
| { | ||
| name: "GroupDelimiter_BlankLine", |
There was a problem hiding this comment.
Would you mind adding a golden test or two for this new option?
This addresses google#97. The discussion there suggested trying to override line breaks, but this turns out to be difficult. The options are not specified until after we've split into lines, and lots of places use line numbers for fixes. Instead, we allow customization of how lines are associated into groups. In order to support the TOML's use case (end groups on blank line) and SQL's use case (end groups if the line ends in a semicolon), we allow a new list of regexes, group_delimiter_regexes that determine if the line should end a group. There's a subtlety to precedence with sticky_prefixes. Since lines starting with sticky_prefixes should associate to a group below them, the first line with a sticky prefix can create a new group, despite what the We allow sticky_prefixes to take precedence.
drubery
force-pushed
the
push-tlymwmkvwxvu
branch
from
e364690 to
c4e3f80
Compare
I had initially shied away from that because we keep a lot of "line numbers" around. Some of those are just internal bookkeeping, but some of those are shown to the user, so those ones need to be real line numbers. But I've just pushed a version with a golden that suggests it might be necessary. My original use case of grouping by double-newline doesn't automatically fix very well because we must put a double-newline at the end of the block. That feels kind of unnatural, and maybe I just need to do the work. I'll try to separate out the two notions of line number and use a line delimiter in the block options instead. |
2 participants
This addresses #97. The discussion there suggested trying to override line breaks, but this turns out to be difficult. The options are not specified until after we've split into lines, and lots of places use line numbers for fixes.
Instead, we allow customization of how lines are associated into groups. In order to support the TOML's use case (end groups on blank line) and SQL's use case (end groups if the line ends in a semicolon), we allow a new list of regexes, group_delimiter_regexes that determine if the line should end a group.
There's a subtlety to precedence with sticky_prefixes. Since lines starting with sticky_prefixes should associate to a group below them, the first line with a sticky prefix can create a new group, despite what the We allow sticky_prefixes to take precedence.