fix(commands): don't indent empty lines - #1653
Conversation
| doc.text(), | ||
| lines.into_iter().map(|line| { | ||
| lines.into_iter().filter_map(|line| { | ||
| if doc.text().get_line(line).unwrap().as_str().unwrap().trim() == "" { |
There was a problem hiding this comment.
Not sure what's the best approach here, initially I was checking that the length of the line is equal to 1 (new line) but that could break for "\r\n". On the other hand, the current approach will skip also lines that already have for example a few spaces (which might be desired).
There was a problem hiding this comment.
Vim indents lines with only whitespace too, but I think it's okay (desirable even) to skip them.
There was a problem hiding this comment.
I have the same opinion but wanted to leave it open for discussion.
There was a problem hiding this comment.
You can use doc.text().line(line) to avoid an unwrap.
Also, https://docs.rs/ropey/1.3.2/ropey/struct.RopeSlice.html#method.as_str
For large slices this method will typically fail and return None because large slices usually cross chunk boundaries in the rope.
Overall, I prefer Cow::from(doc.text().line(line)).trim().is_empty()
There was a problem hiding this comment.
What about in languages where whitespace indentation is meaningful?
If we only check for empty lines then this could be simplified .line(...).len_chars() == 0
There was a problem hiding this comment.
Let's use Cow::from(doc.text().line(line)).trim().is_empty() to avoid the unwrap.
There was a problem hiding this comment.
Updated, thanks for the suggestion.
There was a problem hiding this comment.
I think this is better although it is longer, it does not need any allocations.
if doc.text().line(line).chunks().all(|s| s.trim().is_empty()) { There was a problem hiding this comment.
Nice, thanks for suggestion, updated.
There was a problem hiding this comment.
Seems to still use the old suggestion though
Fixes: #1642