-
Notifications
You must be signed in to change notification settings - Fork 5.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
please treat newlines as terminating a line instead of being a separator #2083
Comments
Hi, I think option to emulate vim behavior should be handled by the code which loads files into the editor. |
Hi, I'm aware of the (unfortunate) incorrect (since it both violates the POSIX standard and causes problems with standard UNIX tools) behaviour of Sublime (et al). For example: a file
The behaviour of vim (et al) is correct and that of Sublime (et al) is not. It is of course possible to have the code that saves (and loads) files handle newlines -- although it would then be nicer to have a setting that allows Thanks. - Felix |
Not all text is meant to be written to files on UNIX compatible OS, e.g cloud9 uses ace as findbar input, where adding newlines isn't proper behavior at all. |
You have a point. However, I consider proper newline handling (for UNIX systems) a fundamental part of being a (cross-platform) editor. Therefore I would prefer ace to (have an option to) handle it. Otherwise everything that uses ace (and wants to properly handle UNIX text files) will have to deal with it separately. I regularly see problems (like incompatibility with shell scripts and other UNIX tools) and annoyances (like I do realize that ace does not deal with files directly. I would consider it debatable whether it makes sense for ace to implement useful features like optionally trimming trailing spaces. There are good arguments for doing so (not needing to implement it multiple times) and not doing so (keeping the complexity of ace to a minimum). But as I said, I consider proper newline handling more fundamental. - Felix |
Why not just type a new line at the end of your file, instead of trying to make ace into the kind of program that alters what the user intends in order to support obscure tools nobody (awesome) uses? |
A diff tool that warns you about not having a newline at the end of your file is telling you to type a newline at the end of your file. It is not telling you that your text editor is broken... I consider spell checking to be indispensable, but to try jamming it down everyone's throat so that ace becomes less useful just to please me... well that would be asinine. |
- Felix |
Please use bold text and all caps when you write the word "optional", it will clear throats and make obvious that this is a feature request. I apologize if you've been offended by my disregard for ancient tech. I assumed you were cracking jokes because you can't even save files in ace without writing your own code to do some post processing. Ensuring that certain file types always end with a new line, and use the proper line breaks for the given platform, is such a trivial task to do when your server gets the post. It is definitely much more sensible to let implementors implement text transformations, and submit pull requests if they create a decent extension. . . Don't you think? You seem quite well informed on the POSIX standards for text files. Couldn't you come up with a simple JavaScript extension for ace that takes the editor contents and manipulated them to suit your needs? I'd applaud you for doing so, and it would look good for you showing off your programming skills instead of your strong opinion of what others should be doing for you. XD hell, I'll even help a bit if you need it. |
@obfusk To implement this behavior from file loading code one needs to do. - session.setValue(string)
+ session.setValue(string.replace(/\r?\n?$/, ""))
- value = session.getValue()
+ value = session.getValue() + session.doc.getNewLineCharacter() if we decide to add option it will become + session.setOption("treatNewLinesAsTerminators", true)
session.setValue(string) which isn't much simpler, but will add much more code into ace for checking the option and making sure switching it on and off doesn't change document value. However the main problem is that this behavior is inconsistent . The only way to select newline character is to move cursor to the next line, but since we do not allow that for the last line, last newline becomes unselectable, and also Another thing to consider is that for simple functionality like this, it is usually easier to reimplement than to find functions provided by ace. e.g. see whitespace.js in ace and same functionality reimplemented then extended in zed. |
My apologies if my request seemed at all like a demand. That was certainly not my intention. I agree that the solution is somewhat trivial and that there is a strong case for doing this kind of processing/validation server-side. Thank you both for taking the time to look at the issue and how it impacts ace. The reason I requested this feature is not that I would not be able to fix it server-side or that I would not be able to write an extension (if I took some time to find some documentation on how to do that) but that I've come across the "incorrect" behaviour multiple times (e.g. when editing files with gitlab or github) and was hoping that fixing it via ace would be a simple way to get the "correct" behaviour (when wanted/needed) in multiple places where ace is used. Especially since it seems that many people using ace are not aware of this issue and will thus be unaware of the potential problems and/or annoyance "incorrect" newline handling can sometimes cause users. I'll try to think of a more appropriate solution (like an extension) and maybe contact gitlab and github as well. - Felix |
@nightwing is there a how-to guide showing how to create a simple extension? |
What I'm currently thinking of is an extension that shows an unobtrusive warning if there's no newline at the end of the file and also integrates with existing settings systems (or maybe just localstorage) to allow a user to specify a preference for either ignoring this in future or always making sure there is one. Not sure how to do that yet though. That how-to might be helpful. - Felix |
Like an icon in the gutter beside the Line numbers? That's unobtrusive and probably wouldn't even need a setting to turn it off. I could see the icon as the symbol on the return key with a "no" (circle slashed out) drawn over it. If the last line isn't "empty" then show the mark. The thing I don't know is whether there is already something in ace for dealing with gutter images, so that if another extension wants to warn on the same line a different image for multiple warnings would show. This should be easy to implement, maybe @nightwing has more information that can help? |
@obfusk i think fixing this behavior via ace won't work because we can't enable that by default, only people that are aware of this issue, will enable the option, also i doubt that github or gitlab will turn this on by default, since many people who are accustomed to sublime behavior, will complain. (I would complain both about unselectable last newline character and about automatically adding newline at the end when i only edit at the top of the file). If you want only to display a warning we can show crossed red circle at the end of the line, like github was doing some time ago, also a behavior to automatically add a newline when writing on empty last line might be useful (it will prevent from accidentally removing last new line, but will still allow to delete it with delete key) there isn't any special documentation about extensions. They either provide some utility functions like ext/whitespace.js or define options |
Agreed with the general consensus here (handle it within each implementation) but thought it might be worthwhile to note that GitHub does automatically append a newline. I don't know when that started or even if it was a change from previous behavior. See, for example, this patch for Caret which I submitted without touching the bottom of the file. |
This issue has not received any attention in 1 year. If you want to keep this issue open, please leave a comment below and auto-close will be canceled. |
Before, a code block would always end with a final newline. This was added by the `hide_lines` function unconditionally. When the code block is syntax highlighted by highlight.js, this is not a problem, no empty line is added for a final trailing `\n` character. However, when the code block is editable and thus handled by the ACE editor, a trailing newline _is_ significant. I believe this issue is most closely described by ajaxorg/ace#2083. The effect of the way ACE handles newlines is that a code block like <pre> Some code </pre> will create an editor with _two_ lines, not just one. By trimming trailing whitespace, we ensure that we don’t accidentally create more lines in the ACE editor than necessary.
Before, a code block would always end with a final newline. The newline was added unconditionally by `hide_lines`. When the code block is syntax highlighted by highlight.js, this is not a problem, no empty line is added for a final trailing `\n` character. However, when the code block is editable and thus handled by the ACE editor, a trailing newline _is_ significant. I believe this issue is most closely described by ajaxorg/ace#2083 in the upstream repository. The effect of the way ACE handles newlines is that a code block like <pre> Some code </pre> will create an editor with _two_ lines, not just one. By trimming trailing whitespace, we ensure that we don’t accidentally create more lines in the ACE editor than necessary.
Before, a code block would always end with a final newline. The newline was added unconditionally by `hide_lines`. When the code block is syntax highlighted by highlight.js, this is not a problem, no empty line is added for a final trailing `\n` character. However, when the code block is editable and thus handled by the ACE editor, a trailing newline _is_ significant. I believe this issue is most closely described by ajaxorg/ace#2083 in the upstream repository. The effect of the way ACE handles newlines is that a code block like <pre> Some code </pre> will create an editor with _two_ lines, not just one. By trimming trailing whitespace, we ensure that we don’t accidentally create more lines in the ACE editor than necessary.
Hi all, Just another data point: I believe I ran into a closely related problem as a user of mdBook: rust-lang/mdBook#1836. There, a Markdown file with ```rust
let x = 10;
``` is turned into HTML like this: <pre>
let x = 10;
</pre> This code block is in turn is turned into an editor with let editor = window.ace.edit(code_block); The resulting editor looks like this: I would have expected the editor to have a single line, not two. The problem is the trailing newline before My changes in rust-lang/mdBook#1836 makes mdBook carefully strip the trailing newline, but I think a better solution would be to let ACE ignore the final newline when creating lines in the editor. |
Before, a code block would always end with a final newline. The newline was added unconditionally by `hide_lines`. When the code block is syntax highlighted by highlight.js, this is not a problem, no empty line is added for a final trailing `\n` character. However, when the code block is editable and thus handled by the ACE editor, a trailing newline _is_ significant. I believe this issue is most closely described by ajaxorg/ace#2083 in the upstream repository. The effect of the way ACE handles newlines is that a code block like <pre> Some code </pre> will create an editor with _two_ lines, not just one. By trimming trailing whitespace, we ensure that we don’t accidentally create more lines in the ACE editor than necessary.
Before, a code block would always end with a final newline. The newline was added unconditionally by `hide_lines`. When the code block is syntax highlighted by highlight.js, this is not a problem, no empty line is added for a final trailing `\n` character. However, when the code block is editable and thus handled by the ACE editor, a trailing newline _is_ significant. I believe this issue is most closely described by ajaxorg/ace#2083 in the upstream repository. The effect of the way ACE handles newlines is that a code block like <pre> Some code </pre> will create an editor with _two_ lines, not just one. By trimming trailing whitespace, we ensure that we don’t accidentally create more lines in the ACE editor than necessary.
Hi,
Ace currently treats newlines as separators, whereas the POSIX standard defines a line as
A sequence of zero or more non- <newline> characters plus a terminating <newline> character.
Most UNIX editors (e.g.
vim
) adhere to this standard. Most unix utilities (likecat
and theread
builtin frombash
) expect files to adhere to this standard. Unfortunately, ace treats newlines differently and both unexpectedly produces files with no newline at the end and displays files that do have a newline at the end as having an extra, empty, line at the end.Given that some people may want to retain the existing behaviour and the fact that the new (IMHO correct) behaviour would not be backwards compatible (i.e. it breaks some of the current tests), I would suggest making the behaviour configurable.
Thanks.
- Felix
The text was updated successfully, but these errors were encountered: