helix
helix copied to clipboard
helix-editor
`:trim` to remove trailing whitespace
Supersedes #3674 and resolves #1481
:trim will remove both trailing empty (lines consisting of only whitespace characters) and trailing spaces on lines. The superseded PR did this as well, but the code became harder to read/reason about (for me) while trying to keep everything in a single transaction, as it allowed you to choose to trim either spaces, lines, or both. This implementation will always trim both.
Edit: I just noticed that it would actually be trivial to reintroduce those options because I ended up implementing this differently than before. It would just be adding lines (would replace var trailing) and spaces as arguments and conditionals to the existing if statements. I won't be doing that though.
I would very much like a config flag to run this command on write. Is that something you would consider adding?
I'll think about it, but at the moment I chose not to so the PR would be simple.
That should be a seperate PR and likely based on #8021. Maybe it doesn't even need a special config flag and instead we can allow users to specify a list of commands to run pre-write
Sounds good. Thanks for the PR by the way. It's a feature I'm really looking forward to. I was actually attempting to do the same thing based on the code from your last PR. But it was slow going as I was trying to learn Rust along the way :)
What about removing leading lines? When working with bigger expressions I tend to add a few empty lines around it to better focus on it, :trim could be useful to clean-up in such cases if it was able to remove leading lines.
Before:
Animals::find().filter(AnimalType::Horse).for_each(|horse|{
horse.feed(
// ⬅️ selection-start
CarrotBuilder::new()
.color(Color::Orange)
.size(Size::Big)
.taste(Taste::Good)
.build()
// ⬅️ selection-end
);
});
After :trim:
Animals::find().filter(AnimalType::Horse).for_each(|horse|{
horse.feed(
CarrotBuilder::new()
.color(Color::Orange)
.size(Size::Big)
.taste(Taste::Good)
.build()
);
});
To me it also makes sense to remove leading spaces:
Before:
horse.feed(⬇️ Carrot::new() ⬇️);
After :trim:
horse.feed(Carrot::new());
That's more or less the job of a formatter. The cases you describe are going to be affected by how the language is defined. For instance, how can you determine whether to trim leading spaces or not when the user might want to keep leading spaces in strings, e.g. " Hello, world!" Or in the case of leading empty lines, whether it fits the coding style? At the point you start going down a rabbit hole of exceptions and cases until you essentially end up with a formatter.
If you really want to do the cases you're talking about without :fmt, you can split your selection to empty lines and leading spaces and then d. :trim is not necessary in that case.
That's more or less the job of a formatter. The cases you describe are going to be affected by how the language is defined. For instance, how can you determine whether to trim leading spaces or not when the user might want to keep leading spaces in strings, e.g.
" Hello, world!"Or in the case of leading empty lines, whether it fits the coding style? At the point you start going down a rabbit hole of exceptions and cases until you essentially end up with a formatter.If you really want to do the cases you're talking about without
:fmt, you can split your selection to empty lines and leading spaces and thend.:trimis not necessary in that case.
Since we want to remove only trailling whitespaces, shouldn't we call this :rtrim: or other name algother? It would be less surprising since most languages I know contains a triplet of funtions: trim, ltrim and rtrim. The trim usually strips from both directions.
See: rust: https://doc.rust-lang.org/std/primitive.str.html#method.trim javascript: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/trim python: https://docs.python.org/3/library/stdtypes.html (called strip)
I don't forsee us adding a trim-leading-whitespace feature so "rtrim"/"rstrip" seems unnecessary to me. If we want to be really clear let's call the command :trim-trailing-whitespace and alias it to :trim.
Would perhaps make sense to sync up with work done in https://github.com/helix-editor/helix/pull/1777 given that EditorConfig have an option for trimming trailing whitespace as well.
I've been running this PR for a few months now. While I don't use it very often, it hasn't caused any issues for me so far.
I'm wondering if we want this or a way to select all trailing whitespace.
Deleting trailing whitespace seems significantly more common than needing to otherwise manipulate trailing whitespace - for anyone who does need to manipulate, I would think that just using \W+$ is suitable.
Could :trim be added and, if it turns out there is a valid usecase for selecting trailing whitespace, just turn :trim into an alias for select+delete?
Yeah I wonder if we really need a typable command for this given you can already select it with %s[ \t]+$ (could be bound to a macro with #4709) and then delete. Automatically trimming whitespace on save could be a nice addition though I think, similar to #8157
Regex that approaches the behavior of this command is something like: (?-m)[ \t]+\n|\s+$. It's not perfectly accurate though since it will always remove the trailing line (EOF). I'm not sure there's a way around that.
@the-mikedavis (or other maintainers) is this something that could be added to the next milestone? Even though there are workarounds (e.g. what is possible with https://github.com/helix-editor/helix/pull/4709), having this out of the box is such a nice quality of life thing. Especially working with any file types that don't have autoformatters.
Does anyone here know how to make a binding to trim trailing whitespace on write? The best I've got is ["select_all", ":trim", "collapse_selection", ":write"] , but this moves the cursor position to the end of the file.
Any updates? It's really quite annoying to have CI complain about trailing whitespace and having to manually fix it line-by-line
A little busy right now. The comment I still need to address wasn't trivial since there isn't an equivalent of Selection::line_ranges for going backwards in the way I needed it to. Or at least I think so, it's been a while so I don't exactly recall what I encountered. It's not difficult but it was enough to have me put this to the side for now. I wouldn't be opposed if anyone wants to take this PR over the finish line themselves though.
In case anyone's interested, I've been working on ec2hx to add the semblance of EditorConfig support to Helix. As part of that, ec2hx now ships with a builtin formatter that just trims trailing whitespace. Even if you are not interested in EditorConfig support, you may want to use this formatter to get just the trimming functionality.
There are pre-built binaries for ec2hx. Once installed, you can add the following formatter config to a language of your choice:
formatter = { command = "ec2hx", args = ["trim-trailing-whitespace"] }
auto-format = true
You can also run ec2hx in a directory with a .editorconfig file with the following content:
[*]
trim_trailing_whitespace = true
That will generate a file .helix/languages.toml the above formatter configuration for all languages which don't already have a formatter or LSP configured. You can copy that to your user configuration.
I hope this is useful to someone. It works well for me on Linux and I have thought about Windows line breaks, but that part isn't tested. It essentially just tries to preserve whatever line breaks the file already has.
