Skip to content
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

Add trim to remove trailing whitespace #3674

Closed
wants to merge 3 commits into from
Closed

Conversation

kirawi
Copy link
Member

@kirawi kirawi commented Sep 3, 2022

Resolves #1481

Currently it doesn't delete empty trailing lines. Is this a desired behavior?

@kirawi kirawi marked this pull request as draft September 3, 2022 22:29
@kirawi kirawi marked this pull request as ready for review September 4, 2022 03:49
@kirawi kirawi changed the title Add trim_whitespace to remove trailing whitespace Add trim to remove trailing whitespace Sep 4, 2022
@poliorcetics
Copy link
Contributor

Currently it doesn't delete empty trailing lines. Is this a desired behavior?

For me yes, but i don't know if others agree

Comment on lines 1552 to 1844
if mode == "all" || mode == "lines" {
let mut lines = doc.text().lines_at(doc.text().len_lines()).reversed();
let mut pos = doc.text().len_chars();

// The last non-empty line char index.
let start = lines
.find_map(|line| {
if line_ending::rope_end_without_line_ending(&line) != 0 {
Some(pos)
} else {
pos -= line.len_chars();
None
}
})
.unwrap_or(0);
let end = doc.text().len_chars();
let transaction = Transaction::change(doc.text(), [(start, end, None)].into_iter());
doc.apply(&transaction, view.id);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does lines do? Does it only trim the last trailing line in the file, or does it trim all newlines? I think the latter is achievable with J

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It trims all the trailing lines in the file. Should I delete this?

Comment on lines +1518 to +1792
fn trim(cx: &mut compositor::Context, args: &[Cow<str>], event: PromptEvent) -> anyhow::Result<()> {
if event != PromptEvent::Validate {
return Ok(());
}

use helix_core::line_ending;

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Command should work on text inside the selection, or if it's a single cursor selection then it should default to the whole file

@kirawi kirawi added A-helix-term Area: Helix term improvements S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 13, 2022
@SoraTenshi
Copy link
Contributor

i am not sure if this may be desired, but an option to trim on :write would be decent i guess?
(obv. also :write-quit)

@kirawi
Copy link
Member Author

kirawi commented Nov 23, 2022

i am not sure if this may be desired, but an option to trim on :write would be decent i guess? (obv. also :write-quit)

Someone could follow up on that with a PR. I'm not personally interested in implementing it.

@SoraTenshi
Copy link
Contributor

i am not sure if this may be desired, but an option to trim on :write would be decent i guess? (obv. also :write-quit)

Someone could follow up on that with a PR. I'm not personally interested in implementing it.

I guess this gonna be me then. :)
Luckily i get a notification once this PR is merged.

@koalalorenzo
Copy link

Is this still being worked on? 👀

@rcorre
Copy link
Contributor

rcorre commented Apr 12, 2023

I'm unsure @koalalorenzo, but as a workaround you can use git-stripspace as a formatter: #1481 (comment)

formatter = { command = "git", args = ["stripspace"] }

That will override an existing formatter, but I figure most language formatters (clang-format, gofmt, rustfmt, etc.) will already strip space, so this is mainly needed for filetypes that don't have formatters.

@Joe-Zer0
Copy link
Contributor

I would eagerly looking forward to a command to trim trailing whitespace (and eventually trim on save as well). Any update on when this will be merged? Thanks!

@tshepang
Copy link
Contributor

oh, didn't think of that (I suppose via :pipe cmd?)... what cmd do you use

@tgross35
Copy link

tgross35 commented Sep 12, 2023

The "trim final newlines" (:trim lines) option of this somewhat relates to #8157 which inserts a final newline if there is none. That is enabled on :w but not :wa, which matches vim.

If this PR gets refactored, enabling "trim trailing whitespace from all lines" (:trim spaces) on :w would likely be a good idea, this would also match vim. :trim lines shouldn't be on by default.

:trim spaces doesn't seem like the most accurate command name, since it doesn't just remove any spaces. Not sure what would be better though: If the full options were named :trim trailing-whitespace and :trim final-newlines that would match vscode (good for discoverability), could have aliases like :trim trailing and :trim lines

@kirawi
Copy link
Member Author

kirawi commented Sep 24, 2023

Superseded by #8366

@kirawi kirawi closed this Sep 24, 2023
@kirawi kirawi deleted the trim branch September 24, 2023 18:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-helix-term Area: Helix term improvements S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add trim_file command and trim_file_on_save configuration option
9 participants