add option to enforce newline at the end of document #5435
Conversation
Xalfer
changed the title
kirawi
added
A-helix-term
There was a problem hiding this comment.
I have also worked on this, but you opened before me the PR, so I wanted to drop some suggestions, hope you won't mind. Also, you should update the book doc as well (you can take a look at my change to see where).
There was a problem hiding this comment.
I think this is a nice option to have. I do find myself editing EOF chars often which show up as changes in git and mess with some tooling. This PR doesn't change the core editing model of helix (all newline remain selecatble and editable) and only acts on save so it's a small non-intrusive option that I think would be a nice addition. I added some comments about the implementation
pascalkuthe
added
S-waiting-on-author
suggested by pascalkuthe Co-authored-by: Pascal Kuthe <pascal.kuthe@semimod.de>
helix-view/src/editor.rs
Outdated
| doc.set_selection(view.id, selection); | ||
| doc.apply(&transaction, view.id); | ||
| doc.append_changes_to_history(view); | ||
| doc.set_selection(view.id, old_selection); |
There was a problem hiding this comment.
I guess you missed my comment https://github.com/helix-editor/helix/pull/5435/files#r1100873864 here since you resolved the conversation too early. You don't need to call set_selection here at all. Just remove all changes to the selection.
helix-view/src/editor.rs
Outdated
| doc.set_selection(view.id, selection); | ||
| doc.apply(&transaction, view.id); | ||
| doc.append_changes_to_history(view); | ||
| doc.set_selection(view.id, old_selection); |
There was a problem hiding this comment.
| doc.set_selection(view.id, selection); | |
| doc.apply(&transaction, view.id); | |
| doc.append_changes_to_history(view); | |
| doc.set_selection(view.id, old_selection); | |
| doc.apply(&transaction, view.id); | |
| doc.append_changes_to_history(view); |
I have applied the same changes in ab0c012 if that's simpler to use.
There was a problem hiding this comment.
I agree with @pascalkuthe here: we shouldn't mess with the selection at all. But otherwise this change seems fine.
I did not realize this before but it was completely useless.
|
Sorry for not doing it earlier I |
| let text = doc.text(); | ||
| let doc_len = text.len_chars(); | ||
| if config.newline_at_eof && get_line_ending(&text.slice(..)).is_none() { | ||
| let view = view_mut!(self); |
There was a problem hiding this comment.
This will fetch the currently focused view which may not be the view that's looking at the document. This can cause a panic with this feature enabled: open a split with two different files without newline endings and :wa.
I think it might make sense to solve this in helix-term::commands::typed in write_impl and write_all_impl rather than here in Editor. We probably also do not want to add a trailing newline when documents are being auto-formatted because we may end up fighting with the auto-formatter (language server or external command)
This resolves helix-editor#4274 with the implementation largely based off of helix-editor#5435 and also addresses the review from @the-mikedavis on that PR. The option name is from EditorConfig's `insert_final_newline`, which is also used by VS Code as `files.insertFinalNewline`. We match Vim's behavior in that :w will add the newline to unmodified files but :wa will not; see helix-editor#1760. Co-authored by: Xalfer <64538944+Xalfer@users.noreply.github.com>
This resolves helix-editor#4274 with the implementation largely based off of helix-editor#5435 and also addresses the review from @the-mikedavis on that PR. The option name is from EditorConfig's `insert_final_newline`, which is also used by VS Code as `files.insertFinalNewline`. We match Vim's behavior in that :w will add the newline to unmodified files but :wa will not; see helix-editor#1760. Co-authored-by: Xalfer <64538944+Xalfer@users.noreply.github.com>
This resolves helix-editor#4274 with the implementation largely based off of helix-editor#5435 and also addresses the review from @the-mikedavis on that PR. The option name is from EditorConfig's `insert_final_newline`, which is also used by VS Code as `files.insertFinalNewline`. We match Vim's behavior in that :w will add the newline to unmodified files but :wa will not; see helix-editor#1760. Tests are included for this. Co-authored-by: Xalfer <64538944+Xalfer@users.noreply.github.com>
resolves #4274