"Remove Trailing Whitespace" should not be on by default

[ceejayoz]

Indented blank lines is a pretty common use case. Having them destructively removed by default is going to lead to a lot of confusion when users inadvertently commit massive whitespace changesets they didn't notice on saving to their Git repos.

[mokkabonna]

Couldn't disagree more. The problem is having those trailing whitespace there in the first place. Best way to deal with this in a project is to run a strip whitespace script on all your files, then commit just that. Clean, simple, solves the problem for the future. Keep the option on by default.

[benogle]

Very strongly agree with @mokkabonna. We would like to keep this default on.

[ceejayoz]

It seems odd to be destructively (as in changes to every file saved without any significant indication it's happening) enforcing coding style preferences as a default. I've worked with a number of folks who like blank lines indented.

That said, not my editor. Thanks for listening, and it's awesome thus far.

[jeffbmartinez]

@ceejayoz was right for at least one person, definitely caused a bit of confusion for me. I like the feature overall, but if you're taking feedback, a one time notification of some sort could be nice so your users know that atom is modifying their files in a non-obvious way.

Either way, keep up the good work!

[pierreozoux]

I would say it's better as an opt-in also... By default I don't like tools to take decisions for me. Just if I configure it...

[clintharris]

+1 for opt-in

[Shnick]

+1 for opt-in

Using https://github.com/zhengjia/rstrip to clean our projects up.

[martindale]

How is this disabled in the first place?

+1 for opt-in

[rustycoat]

+1 for opt-in

Makes a mess of jade files where for logic's sake and the syntax-hilighlighter's sake you want to maintain the indent level even across blank lines. It took me quite a while to figure out that atom was destroying my indents on save...

[rustycoat]

@martindale Atom->Preferences then search for Whitespace on the left to get to the whitespae package's preferenes. Then unclick "remove trailing whitespace."

@ceejayoz is on the money: making destructive changes a default is nonsense.

This is the second 'bug' I ran into in atom within the first couple days of switching to it which seems to come comes form coding-style preferences being unintionally enforced. (The other is atom/text-buffer#11 -- a bug that only affected users who prefer to use hard tabs). Perhaps the development community was not diverse enough in the beginning phases of the project.

[ceejayoz]

@benogle Any chance this can be reconsidered?

[benogle]

We're inclined to leave it the way it is. You can disable it. We'd like to encourage folks to have clean files with no unnecessary whitespace.

[jeffbmartinez]

If you're keeping it as it, it would be nice to have a notification of some sort saying that the file is being modified in a non explicit way. It's just good practice to let users know of features that are not obvious. Personally, I would have never ended up here 6 weeks ago if I'd just known that atom was the one modifying my files without me telling it to do so. I get the feeling this is true for others as well.

As far as the disabling goes, I can't disable a feature I don't know exists.

[rustycoat]

@benogle What you call 'clean files' in this instance is quite problematic for the jade syntax highlighter. If the level of indent is taken away between lines of jade the parser things I've left a block. The :markdown filter is a good test case where this happens constantly.

Best practices vary by language, unfortunately; enforcing them in unexpected ways makes little sense for an editor that aims to be as respectably modular and language-neutral as Atom.

I can very easily make the argument that your 'clean files' are syntactically rather messy in my particular use case.

[barbagallo]

Agree with @hbirch -- @benogle you should open this ticket up and start this discussion again. I, along with my colleagues, all agree that this feature should be an opt-in feature out of the box. To enforce something that is a "nice to have" could potentially cause compilation issues isn't very nice at all.

[myktra]

+1 for the reasons mentioned, esp. w/Jade and Markdown, and having default behaviors not be destructive. Also, IMHO, whitespace handling rules (including things like tab settings) are in the domain/context of the file format and any coding standards associated with the languages typically represented in those formats - not something to be configured globally.

[joeosburn]

+1 for opt-in as well. If the assumed environment is just one developer working on their own project, turning this on by default makes sense. However, most of us using git are working on projects with multiple developers and having the default setting make changes beyond what a developer intends to do (in a particular branch) is not helpful. The default settings should be what is obvious and expected, and not a method to promote a belief on what constitutes a clean file or not (even if I agree wholeheartedly with this setting.)

[pmarchezz]

+1 for opt-in, especially for jade. I just messed up a jade template by not knowing the existence of this option...

[stevennoyce]

+1 for opt-in. I agree that best practices in this field are language dependent, so a destructive global default is not wise. I never see a reason not to remove trailing whitespace that occurs after text on a line, but depending on the language it is not always wise to remove ^\t+ and similar.

[ioquatix]

+100 for opt in.

We're inclined to leave it the way it is. You can disable it. We'd like to encourage folks to have clean files with no unnecessary whitespace.

That's a very subjective definition for "unnecessary". Already had Atom destroy whitespace in several files.

Actually, it would be nice if this could be per-language. But, it would actually be pretty useful if tabs/whitespace settings could be per project or per language.

[Frijol]

+1 for opt-in or per-language support. Like most people here, I use Jade and it breaks my formatting every time I open a Jade file.

[noseglid]

+1 opt in. I don't want my editor doing stuff for me which I haven't specifically told it to do. Least of all altering my precious code.

[ccoenen]

One more for opt-in, please.
I would even go as far as replacing it in favor of something like http://editorconfig.org where you at least can choose this on a per-project/per-filetype basis.

[amcinnes]

+1 for opt-in, because like the vast majority of developers I'm working on a project whose use of trailing whitespace / blank lines isn't perfectly consistent. I don't want to commit unrelated whitespace changes with everything I do. Nor do I want my editor modifying parts of the file that I never touched without telling me. Definitely not by default.

[ioquatix]

I worked around this by disabling the plugin :)

[ioquatix]

The “whitespace” plugin. In the plugin list, search for whitespace, and hit the disable button on the right side panel near the top.

On 13/08/2014, at 1:49 am, Roy Ling notifications@github.com wrote:

@ioquatix which plugin did you disable to avoid this?

—
Reply to this email directly or view it on GitHub.

[50Wliu]

Ideally, I think that the best way to handle this would be to ask the user on first install.

Your suggested compromise seems like a nice start though.

[ghanique]

@sean: yes, dictate is a strong word :-), but is conveys the emotion a
little. An editor should assist me, not make decisions for me. I feel like
this setting is crossing that border. And by doing so it's not assisting me
anymore but actively working against me (by making lots of changes to files
that I did not intended but showing up as changes in the VCS anyway).

When I mentioned that I thought it a weird decision to make this the
default behavior some time ago I couldn't really put in words why I
disagreed. @grecmac did a better job, which is why I gave his message a +1.

I accepted the work around and your decision to have it enabled by default

• it's your product :-). I would disable it. Only changing white space in
  lines that I'm changing would probably also be acceptable, but I can
  imagine that it's more work and not worth the effort. (And there is the
  concern that it conflicts with some markdown dialects where two white
  spaces at the end of the line have meaning).

Kind regards,

Guillaume
On Feb 22, 2015 2:50 AM, "Wliu" notifications@github.com wrote:

Ideally, I think that the best way to handle this would be to ask the
user on first install.

Your suggested compromise seems like a nice start though.

—
Reply to this email directly or view it on GitHub
#10 (comment).

[anaisbetts]

I can't use Atom to work on any existing C# project because it ends up writing dozens of spurious changes into the file because of this.

A better default is "if when loaded the file does not have extra whitespace, clean up extra whitespace the user adds. Otherwise don't remove extra whitespace".

I think this is a compromise that would work really well.

[gregmac]

@50Wliu Asking on install would certainly be better (at least users would be aware), but I don't think it's possible to do and maintain a good user experience.

How do you ask them and explain the implications in enough detail without being overwhelming or annoying? Also what other issues exist that would want a similar dialog (tabs vs spaces, for example), and does this lead to a future where there are dozens of options to pick on startup?

[gregmac]

So just to be clear:

• People that want the setting on by default (4): benogle mokkabonna slang800 50Wliu
• People that expressed explicit support for opt-in (24): ceejayoz pierreozoux clintharris Shnick martindale hbirch barbagallo myktra joeosburn pmarchezz stevennoyce ioquatix Frijol noseglid ccoenen amcinnes bancal laike9m julienpa kangear chesleybrown ghanique virtuald gregmac

There were also a handful of others on this thread that didn't clearly say it should be changed, but did end up on this ticket for a reason.

On the flip side, there are probably many people that either like the defaults or don't care and haven't found or commented on this thread, but I'd counter-argue that people that definitely want their editor to clean up other people's whitespace shouldn't have an issue with explicitly telling their editor that.

---

The argument "for" this (by @mokkabonna) is:

The problem is having those trailing whitespace there in the first place. Best way to deal with this in a project is to run a strip whitespace script on all your files, then commit just that. Clean, simple, solves the problem for the future. Keep the option on by default.

However I'd also argue that this default does not even accomplish this goal!

The current behavior causes unexpected changes on every save. If this is the goal, a better behaviour would be to have a popup when you open a file that says:

There are extra whitespace characters in this file. If you want, they can be 
removed, at which point you should commit to source control before making changes. 

                                  [Leave as-is]   [Never prompt me about this again]
                            [Remove whitespace]    [Always remove without asking me] 

Also to @50Wliu 's comment:

Again, it is easily opt-outable as described above

I'd say it is actually not easy.

Most new users will not be aware a "whitespace" package exists at all (I didn't until I found this ticket), let alone it has settings or default destructive behavior. So a new user has to go to settings, know to click on "packages" (not obvious), know to search for "whitespace" (not obvious) or scroll through all 72 core packages looking for something and take a guess it might be "whitespace" package (again, not obvious).

The setting is called "Remove trailing whitespace" which also doesn't really entirely convey what it's doing, as opposed to something like "Remove all trailing whitespace in file on save".

---

There may be better fixes for the root issue (like @virtuald's suggestion, or mine above) but until PR's exist, I'm strongly in support of merging in @ccoenen's #59.

[50Wliu]

I should clarify-it is easily opt-outable if one is familiar with how Atom handles packages. It will become easily opt-outable in all cases when package and core settings are unified.

I'd argue that "Remove trailing whitespace" is actually fairly accurate, since there is no way to actually do anything to a file without saving it, no?

[notslang]

So just to be clear:

• People that want the setting on by default (4): [...]
• People that expressed explicit support for opt-in (24): [...]

This statistic is misleading because this thread is almost exclusively commented on by users who have noticed the behaviour, didn't like it, and didn't see the whitespace package. Thus, the people on this thread are not an accurate sample of the Atom userbase.

I'd counter-argue that people that definitely want their editor to clean up other people's whitespace shouldn't have an issue with explicitly telling their editor that.

I'd counter-counter-argue that I would hate needing to go through some sort of configuration process (especially a popup). My editor should have reasonable defaults and I should only need to modify settings that I don't like. If there's a setting like whitespace that I don't care about, then I trust the Atom developers to have chosen the best default for me. I simply don't want to have to decide on settings that I don't care about.

However I'd also argue that this default does not even accomplish this goal!

I don't understand how that couldn't accomplish the goal. Unless users are deciding to avoid committing the whitespace removal, and letting those changes sit in their working directory forever. Elaborate?

I'd say it is actually not easy.

Moving the setting into a more visible place, or making the settings easier to search would be nice.

[thedaniel]

The solution most favored by the core team at this time is:

• Try to only strip whitespace from files that are clean when opened (i.e. detect whether it's going to make a huge diff on save and then don't do that)
• Improve settings-view to make this setting easier to discover.

The first item is more complicated and there's no estimate for when the core team will work on it, but PRs are very welcome. I expect the second item will be looked at in the next few months.

[gregmac]

@slang800

My editor should have reasonable defaults and I should only need to modify settings that I don't like.

I totally agree, and the argument I and many others are making is destructive changes are not a good default. Just because this happens to be one you like, you think it's good.

If the default Atom behavior was to switch spaces to tabs, enforce Windows-style line ending (CRLF) and automatically wrap all code at 80 columns, would that also be acceptable?

I don't understand how that couldn't accomplish the goal. Unless users are deciding to avoid committing the whitespace removal, and letting those changes sit in their working directory forever. Elaborate?

You literally described what I was doing before I turned off the whitespace plugin completely.

I open a file in a branch to make a single change, Atom creates a huge diff fixing up whitespace, so I ignore it and commit just the changes I want to make (because nobody will accept a PR with a bunch of style changes on top of a functional change).

If I really wanted to fix the style and remove extra whitespace, I could then switch to master, create a new branch, fix the whitespace, commit and then submit a new PR. And I guess if I'm going to do that, the method using Atom is to open and then immediately save each file one-by-one?

It does not accomplish the goal of fixing whitespace naturally, because developers that pay attention won't commit or accept unrelated changes. And if you do in fact want to cleanup the project and remove extra whitespace, it's a horribly inefficient way to do it.

@thedaniel

The solution most favored by the core team at this time is:

• Try to only strip whitespace from files that are clean when opened (i.e. detect whether it's going to make a huge diff on save and then don't do that)
• Improve settings-view to make this setting easier to discover.

I think that's a very reasonable approach, but I would still like to see #59 merged until that happens.

[ccoenen]

@gregmac i could not have said it any more eloquently and clearly as you have. Thanks for these past three posts.

[anaisbetts]

Try to only strip whitespace from files that are clean when opened (i.e. detect whether it's going to make a huge diff on save and then don't do that)

👍 this works for me

[ccoenen]

"Only stripping on lines that i actually edited myself" AND "for fileformats where i know it's safe to do so"
(combined as in &&) would also be acceptable in my opinion.

[ioquatix]

Destruction of whitespace entered by the user shouldn't be the default in any case.

Try to only strip whitespace from files that are clean when opened (i.e. detect whether it's going to make a huge diff on save and then don't do that)

How is that going to work for a 500Mbyte database dump or log file?

[hbsdev]

I just commited a bunch of files that I edited with atom, only to find out that a lot of insignificant whitespace changes are no in the repository.

Very bad. At least it could have asked me. No more magic please.

[hbsdev]

it is not the editor's place to dictate what my files should look like.

[thomasjo]

I just commited a bunch of files that I edited with atom, only to find out that a lot of insignificant whitespace changes are no in the repository.

While that's unfortunate, it's easily fixed. And for the future, you should pay more attention to what you're committing; it's good practice to have a quick glance at the diff of the stuff you are about to stage (or commit) — git diff is your friend.

---

With regards to the option itself being enabled by default, perhaps we should incorporate some info (with option to toggle the option) in the welcome guide? That way on first launch, new users would be aware of the feature, and can quickly disable it if they decide they don't like it? /cc @thedaniel

[pierreozoux]

Just by seeing the number of people that took time to come here to post something, I believe it is important :) Because, there is also the silent ones! (Ok, I posted more than once here :) )

[vinyar]

As a brand new user I have wasted nearly two hours trying to figure out what's going on, I finally arrived at this page. This left a very sour taste in my mouth. None of the text editors I've ever used destroy hours of your work without asking.

I definitely +500 on not making this a default, especially for files where spaces actually matter. This behavior should most definitely not be enforced in Markdown files.

[notslang]

original text:

I definitely +1 on not making this a default, especially for files where spaces actually matter. This behavior should most definitely not be enforced in Markdown files.
Having spent nearly an hour trying to figure out why my Readme files look terrible, I finally arrived at this page.

@vinyar: I find it hilarious that your comment moved from "nearly an hour" to "nearly two hours", and from "+1" to "+500" between edits.

As for the "hours of your work", it's probably a good idea to save more than once per "hours". Also, hitting ctrl+z will get you back to the point before the whitespace was stripped - it makes a history entry.

[vinyar]

@slang800 Simple - 'an hour' is a figure of speech, but the reality, is that between merging commits, fixing the files multiple times, googling, reading this thread, and filing the issue above it was around 4 hours of literally wasted effort. All in all, I am sorry that your sense of humor regarding usability of a tool leads you to find my wasted effort hilarious.

[afiune]

I just want to second what @vinyar is saying.. I tried Atom and after less than an "hour" I knew there was something wrong. Not nice you joke about other's people time. Be gently. Use https://github.com/SublimeText it is +500 better than this.

Cheers.. 😄

[ghanique]

SublimeText is probably better, but it's also $70. If your employer doesn't
want to spend that money than the Atom Editor with the white spacing turned
off makes your life considerably simpler than many other notepad-clones.

On Mon, Mar 23, 2015 at 7:29 AM, Salim Afiune notifications@github.com
wrote:

I just want to second what @vinyar https://github.com/vinyar is
saying.. I tried Atom and after less than an "hour" I knew there was
something wrong. Not nice you joke about other's people time. Be gently.
Use https://github.com/SublimeText it is +500 better than this.

Cheers.. [image: 😄]

—
Reply to this email directly or view it on GitHub
#10 (comment).

[thedaniel]

Let's not get too far from the topic of the issue. There are better places to discuss the relative merits of editors etc.

[sEver]

So I wanted whitespaces stripped from my *.jade files and I expected the default behavior to be "stripping whitespaces and adding newline at the end of file - enabled".

I found this issue and I found the place to config the whitespace package. The thing is that apparently even though my whitespace package is ENABLED, it still doesn't work in Jade files, because of all the people here pushing against it and the #46 happening.

Would be nice if that particular override happening for Jade files was included in the config file, or hinted at in the whitespace package configuration panel.

Understandable that some people don't like their markup code destroyed, but whitespaces on a single line anywhere else is a bad idea and it's better to have one ugly commit than whitespaces all over the place.

I do believe having clearing whitespaces on by default was a good call. Atom rocks.

[ghanique]

I do see what the problem is with a file where suddenly all the whitespaces are removed and the huge amount of merge conflicts that it creates in the VCS.
I do not see the problem of a file with a lot of whitespaces that I don't see and that do not change the code.

It's not like the whitespaces consume gigabytes of storage and increase the build time to several hours instead of seconds. Merge conflicts, however, can cost hours to resolve. And for what? To remove some characters I cannot see anyway?!?

In some markdown dialects two whitespaces at the end of the line DO have meaning. If they are removed the rendering changes. Great, now the tool DID change my source code...

It would be great to have a menu item that I can click to remove all unnecessary whitespaces, but please do not make that many changes without asking me first!

The workaround it pretty simple: the first thing I do after installing the editor is disable the whitespace thing and then I'm fine. I still don't understand the reasoning, though, why people insist on having it enabled. But then again, who am i
?

—
Sent from Mailbox

On Mon, Mar 23, 2015 at 7:36 PM, sEver notifications@github.com wrote:

So I wanted whitespaces stripped from my *.jade files and I expected the default behavior to be "stripping whitespaces and adding newline at the end of file - enabled".
I found this issue and I found the place to config the whitespace package. The thing is that apparently even though my whitespace package is ENABLED, it still doesn't work in Jade files, because of all the people here pushing against it and the #46 happening.
Would be nice if that particular override happening for Jade files was included in the config file, or hinted at in the whitespace package configuration panel.
Understandable that some people don't like their markup code destroyed, but whitespaces on a single line anywhere else is a bad idea and it's better to have one ugly commit than whitespaces all over the place.

I do believe having clearing whitespaces on by default was a good call. Atom rocks.

Reply to this email directly or view it on GitHub:
#10 (comment)

[thomasjo]

I'm locking this since this will in all likelihood remain on by default, and it's relatively easy to disable;

or by editing your ~/.atom/config.cson file;

"*":
  whitespace:
    removeTrailingWhitespace: false

Note that this the feature can also be disabled on a per-grammar basis.