-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
:trim
to remove trailing whitespace
#8366
base: master
Are you sure you want to change the base?
Conversation
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 :) |
2930921
to
9dd3265
Compare
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, 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 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 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. If you really want to do the cases you're talking about without |
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: See: |
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 |
Just wanted to drop by and say I would love this feature 🙏 |
be72dbb
to
b9f2334
Compare
b9f2334
to
fc296c3
Compare
cf4dc7c
to
7412ff2
Compare
Would perhaps make sense to sync up with work done in #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 Could |
Regex that approaches the behavior of this command is something like: |
Is there any particular reason why this hasn't been merged yet? |
@the-mikedavis (or other maintainers) is this something that could be added to the |
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 vartrailing
) andspaces
as arguments and conditionals to the existing if statements. I won't be doing that though.