Kakoune: Handle trailing whitespaces consistently in all file types (either don't touch them, or always cleanup)

[maximbaz]

I noticed that every language in rc has to deal with trailing white spaces on its own, and ag '(remove|clean).*trailing.*space' --stats shows that there are currently 38 files that clean white spaces. This is a lot of duplicated logic!

With the same motivation "kak is a code editor, and not arbitrary data editor" as mentioned in #2147, I propose to consider making kak always remove trailing white spaces. A simple snippet below should simplify those 38 files and also remove the handling of trailing white spaces from editorconfig parsing:

hook global BufWritePre .* %{ try %{ execute-keys -draft \%s\h+$<ret>d } }

[danr]

I think this is blocked because there is no reliable way to track dependencies between scripts, see mawww's comment here: https://github.com/mawww/kakoune/pull/1811#issuecomment-360634833

[maximbaz]

Well since it can be a simple global BufWritePre hook, we don't really need dependencies between scripts, just put that snippet in e.g. rc/core/whitespaces.kak and start cleaning up all other files that also handle trailing whitespaces 🙂

[lenormf]

Actually the language support scripts I've written do not handle trailing whitespace, as I think it's not the indentation hooks' purpose to handle that.

Secondly, removing all trailing whitespace by default without the user's consent is also bad, the editor shouldn't assume how the user wants a buffer's contents to be formatted.

Example: I regularly have to fix bugs in code that wasn't written by me, that has trailing whitespace everywhere (and even LF and CRLF line endings mixed together), if the editor were to arbitrarily fix those issues, I would end up with diffs that are 99% cosmetic changes every time I close the editor. If the user wants proper formatting, they should use :format or declare hooks in their configuration.

Finally it's not an issue that some code is duplicated across a few files, as the user is free to load whatever script they want, and factorizing redundant code would make things harder to cherry-pick scripts in autoload (c.f. @danr's answer regarding dependencies).

[maximbaz]

There is probably no source code that requires trailing whitespaces to be preserved, git diff -w and modern code review tools are able to hide whitespace differences, so I don't think it's too bad if kakoune would slightly clean code files that you touch. It's a good practice to not have trailing whitespaces in the code, and cleaning them up is not the same as reformatting the file.

Having said that, I'm all for consistency. If kakoune unconditionally adds a missing terminating newline to the file and cleans up trailing whitespaces _sometimes_, it can just as well cleanup the trailing whitespaces all the time — less people in the world will commit code with such kind of issues (yay). Alternatively don't mess with trailing whitespaces in _all_ languages and mention on the wiki how to add the hook.

This is just a proposal, and thank you for commenting and providing great arguments. Good point about mixed LF and CRLF line endings, this is another case to consider. In the meantime, I want to investigate now how to write a hook to fix the mixed line endings 😉

[occivink]

I agree with @lenormf that these hooks don't really belong with the language support scripts, and I'd be in favor of removing them.
Right now doing it unconditionally when leaving insert mode is just too greedy, doing something as simple as pasting an identifier after inserting a newline is frustrating, and doesn't have an obvious workaround.

I also think they're sufficiently trivial to let the users set them up as they want (on a binding, before saving or even automatically when leaving insert mode).

[mawww]

To give a bit of context, I believe the initial language support scripts I wrote do that for a simple reason: They need to cleanup the auto-inserted whitespaces.

When you open a newline, the previous line indent is copied, or maybe the indent gets increased due to something on the previous line. Those inserted whitespaces needs to be removed if we hit <ret> again, or if we quit insert mode with <esc>. The fact that they cleanup the existing text is a side effect, they are mostly there to make the auto-indent behaviour acceptable.

[maximbaz]

This is interesting, question is: do we still want this behavior? ~For example in python this doesn't happen (whitespaces don't get magically removed when I press <ret> again), and nobody seems to be bothered because of this.~ Oh no, it actually happens, I didn't notice at first. After all, :format or people's own hooks to cleanup trailing whitespaces (like I showed in the first message) will take care of them.

So far I have an impression that the consensus is going towards removing any post-formatting logic from syntax files. In my mind we should only keep the hooks that help you _type_, things like increase indent after typing { and decrease indent after typing }. But there should be no hooks that post factum reformat my code — if I added whitespaces and pressed <ret>, they should remain.

[mawww]

Not sure what you meant, if I open a new python file, type def test():<ret><ret>foo the first line of the function is empty, it does not contain trailing whitspaces. The hooks (IMHO correctly) cleaned up the indent it inserted.

[maximbaz]

I updated my message too late, sorry for confusion, yes it happens in python file as well. So I read this as you want to keep this behavior?

[vbauerster]

There is probably no source code that requires trailing whitespaces to be preserved, git diff -w and modern code review tools are able to hide whitespace differences, so I don't think it's too bad if kakoune would slightly clean code files that you touch. It's a good practice to not have trailing whitespaces in the code, and cleaning them up is not the same as reformatting the file.

Having said that, I'm all for consistency. If kakoune unconditionally adds a missing terminating newline to the file and cleans up trailing whitespaces _sometimes_, it can just as well cleanup the trailing whitespaces all the time — less people in the world will commit code with such kind of issues (yay). Alternatively don't mess with trailing whitespaces in _all_ languages and mention on the wiki how to add the hook.

This is just a proposal, and thank you for commenting and providing great arguments. Good point about mixed LF and CRLF line endings, this is another case to consider. In the meantime, I want to investigate now how to write a hook to fix the mixed line endings 😉

It is valid case to have trailing whitespaces in md (markdown) files, which means explicit line break in resulting output. I had bad experience when was making a PR to a md file with "auto cleanup trailing whitespace" option on in my editor.

[eraserhd]

Trailing whitespace in multi-line strings could be intentional, also. (And
multi-line strings are definitely language specific.)

On Sat, Sep 15, 2018 at 4:59 AM Vladimir Bauer [e-mail]
wrote:

There is probably no source code that requires trailing whitespaces to be
preserved, git diff -w and modern code review tools are able to hide
whitespace differences, so I don't think it's too bad if kakoune would
slightly clean code files that you touch. It's a good practice to not have
trailing whitespaces in the code, and cleaning them up is not the same as
reformatting the file.

Having said that, I'm all for consistency. If kakoune unconditionally adds
a missing terminating newline to the file and cleans up trailing
whitespaces sometimes, it can just as well cleanup the trailing
whitespaces all the time — less people in the world will commit code with
such kind of issues (yay). Alternatively don't mess with trailing
whitespaces in all languages and mention on the wiki how to add the
hook.

This is just a proposal, and thank you for commenting and providing great
arguments. Good point about mixed LF and CRLF line endings, this is another
case to consider. In the meantime, I want to investigate now how to write a
hook to fix the mixed line endings 😉

It is valid case to have trailing whitespaces in md (markdown) files,
which means explicit line break in resulting output. I had bad experience
when was making a PR
https://github.com/bahlo/go-styleguide/pull/11#discussion-diff-185476983L105
to a md file with "auto cleanup trailing whitespace" option on in my
editor.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/mawww/kakoune/issues/2175#issuecomment-421542629, or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAI_VAOaQKdarLpL_qCMv83e1siSh9-Dks5ubMGHgaJpZM4U9_De
.

[maximbaz]

I think the consensus now is to not touch whitespaces, also PR #2273 will fix this issue I think.

[occivink]

Just fyi if you don't like the automatic removal of whitespaces when exiting insert mode you can now use
set global disabled_hooks .*-trim-indent

[wcasanova]

is there any way to be able to see the spaces in this way?

[alexherbo2]

add-highlighter global/show-trailing-whitespaces regex '\h+$' 0:Error

[AndrewVos]

@alexherbo2 I've tried that code snippet but it doesn't seem to do anything. Do I need to enable it somehow?