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 config option for clear whitespace line on <ret> #5159

Closed

Conversation

MggMuggins
Copy link

I was unpleasantly surprised this morning to discover that #4854 had been merged in the last release with no discussion.

I find this behavior incredibly frustrating, and most editors don't provide a way to configure it. I have a really nasty regex that works for (n)vim. I've been using kak a bit recently and unfortunately there it's included in the syntax regexes for each language, so I ended up hacking my own versions of those for the languages I use. That's obviously not sustainable. Based on how easy it was to add this config option, I guess this is the "right" way to do it.

I'm not attached to the name for the option, I just really want a way to turn off the behavior introduced in #4854 . I'm happy to figure out how to write a test if that's needed.

I find this behavior incredibly frustrating, and most editors don't provide a
way to configure it. Default is true.
@kirawi kirawi added A-helix-term Area: Helix term improvements S-waiting-on-review Status: Awaiting review from a maintainer. labels Dec 14, 2022
Copy link
Member

@dead10ck dead10ck left a comment

Choose a reason for hiding this comment

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

This seems fine to me, though unsure if there's a better name for the config option.

@MggMuggins
Copy link
Author

Thanks. Like I said, I'm totally open to changing the name of the option if anybody has any suggestions.

@MggMuggins
Copy link
Author

Wondering if somebody else would be willing to review this?

@MggMuggins
Copy link
Author

I hate to continue to shamelessly bump this but I'm still interested in having this merged. I can check/resolve any conflicts if needed.

Copy link
Member

@the-mikedavis the-mikedavis left a comment

Choose a reason for hiding this comment

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

Is there a use-case for turning this off or do you just not like the behavior? I don't see how it could ever be useful to leave trailing whitespace and I haven't seen any traction on that idea in the issue tracker or in the matrix room.

I don't like adding config options that alter behavior in subtle ways like this because it makes things harder to debug, so if this is just personal preference then I'd rather have you carry a small patch (either applied at build-time or in a fork) than us introduce a config option.

@@ -217,6 +217,7 @@ Options for rendering whitespace with visible characters. Use `:set whitespace.r
|-----|-------------|---------|
| `render` | Whether to render whitespace. May either be `"all"` or `"none"`, or a table with sub-keys `space`, `tab`, and `newline`. | `"none"` |
| `characters` | Literal characters to use when rendering whitespace. Sub-keys may be any of `tab`, `space`, `nbsp`, `newline` or `tabpad` | See example below |
| `clear-whitespace-line-on-ret` | If current line is only whitespace, whether to clear it when `<ret>` is pressed | `true`
Copy link
Member

Choose a reason for hiding this comment

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

editor.whitespace is about rendering whitespace with characters like ·. This should live at the top-level of editor instead

@MggMuggins
Copy link
Author

MggMuggins commented Mar 3, 2024

I'm willing to abide this decision; there are good reasons to keep complexity to a minimum in an already very complex tool. That said, I'd like to elaborate on the use case just a little.

I've been using a pair of git filters for about a year now which strip trailing spaces on check-in (see my dotfiles for the full rant; it's going well with the exception of one repo at work where whitespace wasn't managed well). The TL;DR is that trailing whitespace is a pain in revision control, so let's make the RCS ensure the state is consistent instead of forcing the overhead onto the editor/programmer.

I'm having a hard time coming up with any other situation where the editor implicitly removes any text in a file that's being edited; maybe it's just me but I'm not a fan of implicit behavior in general where it can be avoided. I've always found the "insert whitespace and then delete it right away" a little perplexing.

</rant>

EDIT: Also I joined the matrix (@MggMuggins:matrix.org) so ping me if you want to discuss more there. I didn't see any scrollback discussing this PR.

@MggMuggins MggMuggins closed this Mar 3, 2024
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-review Status: Awaiting review from a maintainer.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants