Skip to content
This repository has been archived by the owner on Dec 15, 2022. It is now read-only.

Regarding trailing whitespace.. #77

Closed
ghost opened this issue Jun 24, 2015 · 10 comments
Closed

Regarding trailing whitespace.. #77

ghost opened this issue Jun 24, 2015 · 10 comments

Comments

@ghost
Copy link

ghost commented Jun 24, 2015

My git commits were being mangled for a few months and I've finally discovered why: #10

It should not take so much effort to discover the editor is silently changing things. Someone says the codebase should change. Unfortunately that's not an option, it's not my code base it's my employers.

If atom is going to be taken seriously it's default plugins can't silently mangle code to meet the authors style preference. Any destructive modifications should at the very least ask. And yes, removing arbitrary characters because the author doesn't like them is destructive.

I realize I'm free to use any editor. I like everything else about atom and now I know how to disable this. That's not the point, and as others stated some languages literally break when whitespace is removed

Please make this way more obvious. Or disabled by default. Or even a popup that says "Do you want Atom to enforce it's own style on your code regardless of your employers requirements"

@winstliu
Copy link
Contributor

I'm going to quote this from @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.

Sorry about your issues with the whitespace removal, but this option is still probably going to stay on by default (per @thomasjo's last comment in #10).

@ghost
Copy link
Author

ghost commented Jun 24, 2015

My main issue is that by going with "but this option is still probably going to stay on by default " is you have to know it's an option to disable it

Imagine if notepad stripped semicolons because microsoft didn't like semicolons. That's how a lot of people feel about this.

@thomasjo
Copy link
Contributor

I agree that these options should likely have been disabled by default (unless it was combined with a only strip whitespace on lines that have already been changed option of some kind), but I'm not sure we can really change it until after Atom v1.0 ships. That being said, if someone can land a PR for the "changed lines" option, we might be able to sneak it in fairly soon after v1.0.

I really wish we could just change the defaults, but that would be considered a "breaking" change at this point, hence the reason for the new option.

/cc @nathansobo @benogle @kevinsawicki

@ghost
Copy link
Author

ghost commented Jun 24, 2015

I'm curious how it would be considered a "breaking" change;

With it enabled, it makes style changes at the expense of issues with syntax highlighting, git diffs and markdown.

With it disabled, it simply doesn't do anything.

I'm having a hard time understanding how "not breaking things" is a "breaking change"

@thomasjo
Copy link
Contributor

Simple; it's a "breaking" change since it's been a default for a very, very long time (since before Atom was made publicly available), and since we're in lockdown mode now for v1.0, we cannot simply change longstanding defaults at this point in time.

@ghost
Copy link
Author

ghost commented Jun 24, 2015

With all due respect, that sounds like "we're going to leave it broken because that's the way it's always been." I simply can't accept that destructively and silently modifying files ought to be the default behaviour.

I can't think of a single case where disabling that by default would affect anyone negatively. There are lots (see issues #10, #43) where leaving it on does.

If people depend on it, for some reason that I can't fathom, they could just turn the setting on. That is, opting in to silent, destructive modification of files.

@gregmac
Copy link

gregmac commented Jun 25, 2015

All due respect, #10 was opened nearly a year and a half ago, and there was plenty of time to change the defaults then.

@r-chris
Copy link

r-chris commented Jul 12, 2016

I see a possible solution that could solve this whole conflict. Keep it on by default, but ask/inform the user the first time it happens:

  • on file save show "Atom will remove all trailing whitespaces. [OK] [Undo] [x Remember?]"

The main problem folks seem to have is that they don't know this is actually on by default. It's the same concept applied to saving passwords in browsers. They don't just save it for you - they ask the user and to make it less annoying you can just remember the decision and will only see the Notification once.

Furthermore, this could possibly be disabled by default for certain languages where it is a known problem (or you could ask again the first time this sort of file is opened).

Lastly, there is an option to not remove whitespaces on the current line, but what is missing is the suggested removal of whitespaces only on edited lines.

@JamesJDai
Copy link

JamesJDai commented Nov 23, 2017

I have read this discussion from "Remove Trailing Whitespace" should not be on by default #10 Do not automatically trim end of line white space #27. I have never seen this on any other editor. And this should really be a selectable option and not on by default. This has really driven me crazy as I search for the cause.

@ghost

This comment has been minimized.

@atom atom locked as off-topic and limited conversation to collaborators Aug 9, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants