"Remove trailing spaces (on Save)" option in the editor just broke in 1.9.0.dev_09_01

[DartBot]

This issue was originally filed by @a14n

---

On ubuntu 14.04

Actually, it's not always broken.

Let's start with no trailing spaces :

---

abstract class JsFoo {
  /// test
  JsFoo();

  int i, k;
}

---

Insert 4 spaces at empty line 4 :

---

abstract class JsFoo {
  /// test
  JsFoo();
    
  int i, k;
}

---

Save. The spaces are still there.

Add a space at the end of another line (line 1 for instance).

---

abstract class JsFoo {
  /// test
  JsFoo();
    
  int i, k;
}

---

Save. All the trailing spaces are removed as expected. Strange!

[DartBot]

This comment was originally written by @a14n

---

It seems the concern appears only when your last changes was adding/removing whitespaces on lines with only whitespaces.

[kasperl]

cc @danrubel.
Added this to the 1.9 milestone.
Removed Priority-Unassigned label.
Added Priority-High, Area-Editor, Triaged labels.

[DartBot]

This comment was originally written by [email protected]

---

Only whitespace in front of the caret (in the currently active line) is preserved if the line contains nothing but whitespace.

E.g. this:

    <space><space>|<space>

becomes:

    <space><space>|

This is actually the desired behavior. See #­22179 for details:

https://code.google.com/p/dart/issues/detail?id=22179

[DartBot]

This comment was originally written by @a14n

---

Thanks for the link to the original issue.

I really don't like this new behaviour.

First the workflow explained in the issue #22179 is strange to me. I never needed to save a file after just adding only 2 whitespaces and before adding meaningful content.

Most importantly this new behaviour is really boring when you edit any file. For instance I have a class to which I need to add a method or field.

---

class Foo {
  Foo();

  int i, k;

  int m2() => null;
}

---

Now I add a getter and press enter at the end of the getter to insert a blank line before m2 :

---

class Foo {
  Foo();

  int i, k;

  String get g => null;
  
  int m2() => null;
}

---

After pressing Enter at the end of the getter the caret is prepended by 2 whitespaces. When you save the file this 2 whitespaces are saved too. This is not what I wanted.

Comparing the 2 aboves use-cases it is most likely that the second occurs more often. That's why I think this change should be reverted.

[danrubel]

Thoughts?

---

Set owner to @lukechurch.

[DartBot]

This comment was originally written by [email protected]

---

Netbeans, Komodo, and IntelliJ/WebStorm keep the before-caret whitespace of the current line just like Dart Editor currently does. (Visual Studio has no option for removing trailing whitespace on save.)

While I do agree that this behavior is a tad quirky, it's what everyone else is doing and I guess it's more convenient if the indention level of the current line is kept. After all, starting to type at the beginning of a line won't auto-indent it.

[DartBot]

This comment was originally written by @a14n

---

My main concern is that the files will end up with trailing whitespaces almost at every save despite the fact that I checked "Remove trailing spaces".

Moreover if you commit with those whitespaces the next change on file will produce a useless diff showing that some whitespaces have been removed.

[DartBot]

This comment was originally written by [email protected]

---

In Netbeans, I use a theme which highlights trailing whitespace with a pink strike-through line. I kinda like that, but I'd imagine that most people would find it visually distracting. It's a bit "chatty". Even if you use a color which blends into the background.

---

Attachment:
netbeans-trailing-whitespace-theme.gif (12.96 KB)

[DartBot]

This comment was originally written by [email protected]

---

Looks like the animation doesn't work if you use "view". Guess you have to download it. Sorry about that.

[clayberg]

cc @lukechurch.
Set owner to @scheglov.

[scheglov]

Luke, it is implemented as https://code.google.com/p/dart/issues/detail?id=22179 requested.
Should we roll it back?

---

Set owner to @lukechurch.

[clayberg]

This is working as intended.

---

Added AsDesigned label.

[DartBot]

This comment was originally written by @a14n

---

AsDesign ????

I agree this works (almost) as requested in issue #22179 but this is really a bad move IMHO. Moreover the request has been done by only one person since the public release of Dart. Why wasn't it this issue #22179 that has been closed AsDesign ?

It's really strange to implement this issue at the last release of the dev channel before a stable release without any more feedback. It's boring in my dev workflow and I think other will have the same problems too.

Since this change landed in the dev channel 4 days ago I end up at every save with whitespaces in my files and it drives me crazy to remove them to have a clean file to commit. That's no sens since I checked "remove trailing whitespaces".

[DartBot]

This comment was originally written by [email protected]

---

Sorry it's frustrating for you.

Here's the problem - the alternative was also driving other users crazy. We can't have save causing the cursor to jump. People hit save all the time.

You hit new line, the editor would insert a new line, the user would hit save and the cursor would jump left, they would then have to backspace up to the previous line and re-add it. This means that save is a distracting operation, which is precisely the opposite of the save, everything will be ok whenever you do this operation that save is supposed to be.

If we keep it like this the documentation should be updated to: "Remove whitespace on save except when it's the only thing on the line"

This is consistent with other tooling.

=== Looking at alternatives.

If I understand the use cases for this correctly, it's mainly because of the need to check it into source code. So an alternative strategy would be to turn the white space stripping back to the aggressive behaviour and then also give people a right click operation so that people can manually request white space stripping on the specific files that they have selected whilst having it off on save.

Ideally our formatter would be able to do that, as a 'always run this before you commit and your code will be fine', but it tends to be a bit destructive right now.

What do you think?

[clayberg]

Luke: did you mean "Remove whitespace on save except when it's the only thing on the line with the cursor"?

[bwilkerson]

Personally, I don't like having whitespace on otherwise empty lines, but that's a distant secondary concern.

The biggest concern I have with this is that it sounds like it could lead to a lot of whitespace churn when looking at diff's. Of course, if the formatter also runs automatically on save, then we'd be back to having the cursor jump.

I suppose we could special case this and remove trailing whitespace everywhere except the line containing the cursor if the whitespace is before the cursor. That would at least minimize the churn.

[scheglov]

Personally, I don't like this feature too.
So, I'm restoring removing all trailing spaces.

https://codereview.chromium.org/975483003

---

Added Started label.

[scheglov]

cc @lukechurch.
Set owner to @scheglov.

[danrubel]

+1 restoring this to the original behavior