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

Setting "Ensuring Single Trailing Newline" is broken - the trailing newline is misleadlingly shown in the editor as advertised, but not actually saved in the file on disk #144

Open
1 task done
ghost opened this issue Jan 16, 2017 · 41 comments

Comments

@ghost
Copy link

ghost commented Jan 16, 2017

Prerequisites

Description

The whitespace package setting "Ensuring Single Trailing Newline" appears to be broken: the trailing newline is misleadlingly shown in the editor as advertised, but not actually saved in the file on disk (which the description text seems to suggest, and which would IMHO be a much more reasonable behavior).

Since this is annoying for people using something like emacs/vim and I generally have a single trailing newline in my project which Atom is now breaking when I use it, it would be nice if this could be fixed.

Steps to Reproduce

  1. Check the Atom settings for the whitespace package and ensure "Ensuring Single Trailing Newline" is enabled
  2. Open any non-empty source code file which has a single trailing newline in Atom
  3. Hit CTRL+S
  4. Open the file you just saved in vim/emacs/.. and observe there isn't any trailing newline anymore

Expected behavior: with "Ensuring Single Trailing Newline" enabled, I don't just see a newline in Atom but it's actually saved to disk as well

Actual behavior: no trailing newlines saved to disk, which is a problem when collaborating with other editor users to whom this is potentially an annoyance

Reproduces how often: 100% (every time)

Versions

[e@e ~]$ atom --version
Atom    : 1.10.2
Electron: 0.37.8
Chrome  : 49.0.2623.75
Node    : 5.10.0
[e@e ~]$ apm --version
apm  1.12.5
npm  3.10.5
node 4.4.5
python 2.7.12
git 2.9.3
[e@e ~]$ uname -a
Linux falcon 4.8.16-300.fc25.x86_64 #1 SMP Fri Jan 6 18:11:49 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux
[e@e ~]$

Additional Information

Any additional information, configuration or data that might be necessary to reproduce the issue.

@ghost
Copy link
Author

ghost commented Jan 16, 2017

As it appears, all major Linux editors (gedit / vim / ..) actually don't show the last line break ("\n" / ascii 0x0A) at the end because apparently POSIX says they shouldn't.

Therefore, this seems to be composed of two bugs:

  1. Atom should fix its rendering to conform to the POSIX standard (yes I think the standard is silly too regarding this, but everyone else adheres to it) - already filed as bug Suppress display of end-of-file newlines as blank lines atom#7787

  2. "Ensuring Single Trailing Newline" should actually insert two ascii 0x0A 0x0A for all files with Linux line breaks, because that results in what the user expects to see: a single, visible newline at the end.

    It doesn't really matter if technically that's two line break characters, because in the end it's still one empty line and neither vim nor gedit nor GitHub's web interface nor <insert arbitrary linux editor> actually show a single line break character 0x0A at the end (and in fact you can't save without such a hidden line break anyway), so the user expectation for a trailing newline is clearly to terminate all files with 0x0A 0x0A after stripping trailing whitespace. (and to show that as a single newline in the editor)

@winstliu
Copy link
Contributor

winstliu commented Feb 5, 2017

No updates. I work on Windows, but almost everyone else on the Atom team uses macOS.

/cc @ungb for reproduction.

@ghost
Copy link
Author

ghost commented Feb 5, 2017

Just to make the ticket more clear, since I am throwing expectations of visible newlines and actual "\n" a bit willy nilly together:

What "Ensuring Single Trailing Newline" seems to do right now is:

  • result in a trailing newline on Windows that shows up in editors
  • ??? on Mac OS X
  • result in no visible trailing newline on Linux that would show up in any editors, but it does add a proper POSIX hidden newline (which is actually visible purely in Atom although it shouldn't be, thanks to Suppress display of end-of-file newlines as blank lines atom#7787 - but not in any other editors)

At this point I am not even sure what it is supposed to do, but I suggest it should be some sort of coherent choice of visible new lines on all platforms, e.g. 1 consistently visible empty line at the end of the file on all platforms.

@jeffrifwald
Copy link

Any updates here? I won't be able to use Atom based on this issue alone.

@shelldandy
Copy link

@jmcriffey you can just disable the whitespace plugin which is what I did

@ghost
Copy link
Author

ghost commented Sep 26, 2017

Yea it is pretty annoying though since whitespace is enabled per default, so if you work on a bigger project with contributors all of them will mess with your project's whitespace too even if you disabled it for yourself. It would be a lot better if this could be finally fixed.

@ungb
Copy link

ungb commented Sep 26, 2017

I can repro this issue on mac. Very sorry for the delay in response as I just got to this on my backlog.

I will bring this issue up with the team.

issue

@maxbrunsfeld
Copy link
Contributor

maxbrunsfeld commented Sep 27, 2017

@ungb That GIF appears to show the correct behavior: after saving the file in Atom, it ends with a single newline.

@Jonast I can't reproduce any problem with this. Can you enable the Whitespace package, save a small file in Atom, and show me the contents of the file as reported by hexdump?

For example, if you create a file containing just the word hi, it should appear in Atom like this:

1| hi
2|

Note that Atom renders the single empty line at the end of the file, unlike Vim. This is intentional. In Atom, you have the freedom to disable the Whitespace package and you will then be able to save files without trailing newlines. Then they will appear like this:

1| hi

Because we support the ability to not have a trailing newline, it would not make sense for us to hide the trailing newline, because then you wouldn't be able to tell visually if there was a newline there or not. This is why we show it.

Anyway, if you save the file in Atom with the Whitespace package enabled, you should see this in your terminal:

$ hexdump test.txt
0000000 68 69 0a                                       
0000003

Is that not what you are seeing?

@maxbrunsfeld
Copy link
Contributor

Could you give me a link to the paragraph in the POSIX standard that talks about these semantics?

Also, if this rendering difference is all that you're talking about, then this issue is an exact duplicate of atom/atom#7787, and has nothing to do with the white space package.

@maxbrunsfeld
Copy link
Contributor

maxbrunsfeld commented Sep 28, 2017

I'm actually going to go ahead and close this out. The way you originally reported the issue, it made it seem like the file was being saved to disk without a trailing newline, but it sounds like you're actually just talking about the rendering issue we mentioned above.

If you are actually seeing some problem with the on-disk contents of the file's we're saving, we can reopen.

@ghost
Copy link
Author

ghost commented Sep 28, 2017

The issue IS NOT saved with a trailing newline, as proven by your own hex dump. Please reopen.

The only reason it LOOKS like it has a trailing newline is because of a render bug.

@maxbrunsfeld
Copy link
Contributor

maxbrunsfeld commented Sep 28, 2017

No. The hex dump I reported shows that the file is being saved correctly. It ends with a 0a. That is the newline character.

@ghost
Copy link
Author

ghost commented Sep 28, 2017

Here are the sources:

Relevant POSIX section: http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap03.html#tag_03_206 (please note this implies that characters + \n means there is no empty line following, also see below)

http://c0x.coding-guidelines.com/5.1.1.2.html#123 (C standard interpretation which is more concrete and followed by all editors - which basically says \n at the end is always present and therefore this doesn't imply an additional new line by itself)

Again, this means ending with a 0a means the file has no empty line following which the whitespace package implies, which also means no visible new line at the end which is what the package does on all other platforms and consequently also should do on Linux.

@ghost
Copy link
Author

ghost commented Sep 28, 2017

Basically IMHO you should completely forget about the meaning of \n itself.

Instead, whitespace behavior should be based on actually visible new lines (unrelated to \n) at the end of the file, since that is what the user sees. Consequently, if it is supposed to add an empty line at the end, this must translate to \n\n on Linux. If it's not supposed to have an empty line at the end, just end with \n as it does now. Otherwise the behavior will be visually inconsistent to all other platforms, which IMHO makes absolutely no sense at all.

TL;DR: make up your mind about what is actually supposed to be the visible end result, and stop defining this over detailed new line character semantics

@maxbrunsfeld
Copy link
Contributor

The name of the setting is 'Ensuring Single Trailing Newline'. This name is 100% correct. 'Newline' is a generic term that means a character sequence that represents a line ending. On posix platforms, the character sequence is \n. The whitespace package ensures that the file ends with a single \n character - a single trailing newline.

If you would like to further discuss whether or not we should show the final line number, please comment on atom/atom#7787.

@ghost
Copy link
Author

ghost commented Sep 28, 2017

I think as you describe it, the setting would be useless.

Consider the following:

  1. A user saves a file on Windows with \r\n and a trailing new line in Atom with whitespace

  2. This is opened by another user on Linux with VIM and converted to Linux whitespace

  3. Now if this file is opened in Atom again, the whitespace package will consider it different (beyond the pure conversion, with an actually different amount of lines)

This makes no sense at all.

Also, IMHO atom/whitespace is most useful to the users to ensure a consistent display since whitespace is mostly about unified visual consistence and styling, that is something a style checker would complain about. Therefore, a consistent visible amount of newlines is IMHO way more relevant than what happens on a hex dump level.

A useful setting would therefore be "Ensure Single Trailing Visible New Line". (Edit: or alternatively considering "Newline" as a visual element and not as the hex dump level new line character \n as you currently do)

Why again are you so absolutely dead-set on hidden whitespace semantics nobody actually sees, and nobody cares about? The fact that Atom has this unintentional display difference because this was missed by so many people should already show that this is a technical underlying detail that has no place in an options dialog, and which no regular user should be concerned about.

Again just my humble opinion, but I think your approach here is nitpicky in a way that absolutely hurts the user experience and isn't actually useful. (beyond argueing "but the setting is already named in such a nitpicky way that this interpretation is technically correct" - it might be, but does that help anyone in practice?)

Sorry for ranting a bit, I am just trying to illustrate in sufficient detail why I think your position on this should change. I will also refrain from commenting here further since I don't think I can add more. Just please take some time to think about this and whether the current behavior is really useful, and what would be most useful for the users to have..

@maxbrunsfeld
Copy link
Contributor

maxbrunsfeld commented Sep 28, 2017

  1. Now if this file is opened in Atom again, the whitespace package will consider it different (beyond the pure conversion, with an actually different amount of lines)

No, I don't think it will show a different number of lines. Converting a file from CRLF line endings to LF line endings will not affect the line count that Atom displays. If you really are seeing this, please record a GIF and open a separate issue for it, as that would be a bug.

@ghost
Copy link
Author

ghost commented Sep 28, 2017

@maxbrunsfeld it does:

Here the concrete demonstration of the three bug steps above:

  1. Make a file in gedit with trailing new line and save with Windows encoded line breaks:

step1

  1. See what wc -l says on it, and convert it to Unix line breaks:

step2

  1. See what it looks like in Atom:

step3

  1. Make sure whitespace is enabled and save the file, then check wc -l:
    step4

As you can see, atom/whitespace changed this file.

That is also the entire point of my rant. And that's also why I think having a setting "append a newline character" (current implementation) is useless to have - what people want is "append a visual new line" (what I am suggesting with this ticket, which requires \n\n on Linux at the end).

@maxbrunsfeld
Copy link
Contributor

Ok, again, this is exactly the expected behavior. It has nothing to do with whether you're using CRLF line endings or LF line endings.

The whitespace package ensures that the file ends with A Single Trailing Newline. Not two trailing newlines; a single trailing newline. You explicitly created a file with two trailing newlines in gedit. Then, when you saved the file in Atom, Atom removed the second trailing newline.

And that's also why I think having a setting "append a newline character" (current implementation) is useless to have - what people want is "append a visual new line".

No, I don't think that is what most people want. That is not the built-in behavior of most editors.

  • The built-in behavior of most editors is to ensure that a file ends with a newline. The whitespace package ensures this.

  • Another closely-related thing that most people want is to avoid extra newlines at the end of a file. The whitespace package ensures this as well.

@maxbrunsfeld
Copy link
Contributor

I really do understand why you care about the line number rendering issue (atom/atom#7787), and I understand that that issue has been ignored. But I really do not think there is any problem with how Atom is saving, loading, or editing these files, and I think the names of our settings are accurate. If you have more UX thoughts on this to share, please do so on that other issue.

@ghost
Copy link
Author

ghost commented Sep 28, 2017

Well but doesn't it add an extra visible \r\n on Windows? (which is actually visible, and on Linux it will just add a single \n which won't be visible in any reasonable editor outside Atom)

@Ben3eeE
Copy link

Ben3eeE commented Sep 28, 2017

The setting works exactly the same way on Windows. The setting only ensures that the POSIX standard is followed and is on by default.

We do render the line and that is another issue atom/atom#7787

@ghost
Copy link
Author

ghost commented Sep 28, 2017

@Ben3eeE so on windows, no trailing \r\n will be added? (since that would be visually consistent to a single \n on Linux) Sorry if I'm asking obvious questions here, but I don't have a Windows running right now.

@glen-84
Copy link

glen-84 commented Sep 28, 2017

You're confusing the issue by talking about what is "visible", because this differs based on the editor.

This setting just ensures an EOF newline sequence. In Atom, this is currently displayed as an "empty line" – in other editors (vim, etc.), it's not.

@Ben3eeE
Copy link

Ben3eeE commented Sep 28, 2017

The setting does:

  • Remove extra empty lines in the end of the file
  • Add \n to the last line OR add \r\n to the last line. I'm pretty sure that this depends on what is configured by the line ending selector.

So on Windows it would work exactly the same way as on Linux except it would ensure that the last line ends with \r\n if you open a line with Windows line endings.

If you disable this setting then you can save files that don't follow the POSIX standard. I believe other editors also have a setting for this.

We do render the line, which might be confusing.

@maxbrunsfeld
Copy link
Contributor

Atom will detect the line ending that a file is using and adopt that line ending, regardless of your platform. The only time that the platform makes a difference is when you're creating a new file. In that case, Atom will use your platform's line ending.

@ghost
Copy link
Author

ghost commented Sep 28, 2017

Well but in that case it will actually be visually inconsistent between Windows (visible empty line at the end) and Linux (no visible empty line at the end) once atom/atom#7787 is going to be fixed, no?

You're confusing the issue by talking about what is "visible", because this differs based on the editor.

It doesn't differ based on editor, just based on platform. (unless the editor is broken, which most others than Atom aren't purely regarding this issue)

The setting works exactly the same way on Windows.

That is exactly my point. It won't once atom/atom#7787 is fixed based on what the user can see. Of course if you hex dump dive into this it will be a comparable result, but is this relevant to the user? Also it's currently already inconsistent (visually) if you open it with any other linux editor than Atom (no visible empty line at end), compared to what you see with Windows line breaks on Windows in Atom or Notepad, .. (visible empty line at end)

@Ben3eeE
Copy link

Ben3eeE commented Sep 28, 2017

Why do you think it would be visually inconsistent?

@ghost
Copy link
Author

ghost commented Sep 28, 2017

Because \n at the end won't show up on Linux (no trailing empty line) in any reasonable editor, but \r\n shows up as empty line at the end on Windows (trailing empty line).

Therefore, \r\n at the end of the file on Windows is equivalent to \n\n at the end of file on Linux - purely visually of course.

@Ben3eeE
Copy link

Ben3eeE commented Sep 28, 2017

How does a file ending in \r\n on Linux render in other editors?

@ghost
Copy link
Author

ghost commented Sep 28, 2017

It does actually not render as a separate trailing empty line in the Linux editors I checked, but that is a bug (since in Windows editors it WILL show up as a separate trailing empty line).

Here is now a file with a single trailing \r\n shows up in Notepad on Windows 10 right now:

notepad

This is how the exact same file shows up in Vim on Linux (without line space conversion):

vim

@glen-84
Copy link

glen-84 commented Sep 28, 2017

It doesn't differ based on editor, just based on platform.

Yes it does – Atom shows a line, and vim doesn't.

FWIW, VS Code also shows the line, as will a number of other editors.

@Ben3eeE
Copy link

Ben3eeE commented Sep 28, 2017

That's a good point. We should consider it when implementing atom/atom#7787 and ensure that we follow the correct practices on how this should be rendered on Windows. We don't want to not render the line if it should be rendered.

Thank you for your comments @Jonast 🙇 Can you add a comment about the \r\n issue on Windows in atom/atom#7787?

@glen-84
Copy link

glen-84 commented Sep 28, 2017

but that is a bug

No it's not. \r\n will only terminate the def line.

@glen-84
Copy link

glen-84 commented Sep 28, 2017

@glen-84 it only does so because this is broken on Atom. Virtually on other Linux editor behaves like that.

I just mentioned VS Code, and I'm pretty sure that there are others (mostly GUI-based).

@ghost
Copy link
Author

ghost commented Sep 28, 2017

It is a bug, since a single trailing \r\n actually shows up as a trailing empty line on Windows:

notepad

Or are you saying Windows editors are displaying Windows line breaks incorrectly?

@glen-84
Copy link

glen-84 commented Sep 28, 2017

vim follows POSIX, so each line must end with a newline.

Windows editors (if I'm not mistaken), do not.

So, there is no bug.

@ghost
Copy link
Author

ghost commented Sep 28, 2017

The bug is that if a file with windows line breaks is shown, the Windows line breaks conventions should be followed. Or do you disagree? Otherwise, WIndows text files transferred without any alteration will show a different amount of lines in a Linux editor exhibiting this bug, although actually being the exact same file.

@glen-84
Copy link

glen-84 commented Sep 28, 2017

vim etc. don't care about other operating systems, it shouldn't have to handle all cases. It only "knows" about \n, because it runs on Linux.

@ghost
Copy link
Author

ghost commented Sep 28, 2017

I guess that could be one way to see it since Vim isn't cross-platform, but that doesn't necessarily make it a feature. If at all it would be an accepted limitation, but not correct behavior for treating Windows line breaks..

@ricardofunke
Copy link

IMHO If Atom is in a Posix environment, it should follow the Posix environment. So if it saves the file in Posix format, it should show the file as it was saved, i.e in Posix format.

Moreover if other editors don't show the trailing newline to follow the Posix standard, Atom should just do like them.

This is consistency in my opinion. So why should we have an Editor that always shows to the user a line that doesn't exist in the saved file and in any other editor that reads that file in the same environment?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

8 participants