Skip to content
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

bug: insert-final-newline should only insert if file non-empty #11967

Open
Axlefublr opened this issue Oct 30, 2024 · 5 comments
Open

bug: insert-final-newline should only insert if file non-empty #11967

Axlefublr opened this issue Oct 30, 2024 · 5 comments
Labels
A-command Area: Commands

Comments

@Axlefublr
Copy link
Contributor

I have the solution ready and am ready to PR it, but I'd want to first ensure this is wanted by the maintainers;

The insert-final-newline option ensures the final newline always, even if the file is empty otherwise.
With this option enabled, you effectively cannot save an empty file, which is silly.

Do you also consider it a bug? Or is it somehow intended behavior?

@the-mikedavis
Copy link
Member

I don't have a strong opinion on this - I would usually use touch for this instead.

@the-mikedavis the-mikedavis added the A-command Area: Commands label Nov 6, 2024
@verte-zerg
Copy link

I would like to see this fix, as I’ve noticed that Helix sometimes adds an extra line to __init__.py files, which should almost always be empty. Many formatters, at least in Python, don’t add a final line if the file is empty.

@Axlefublr
Copy link
Contributor Author

Axlefublr commented Nov 7, 2024

diff --git a/helix-term/src/commands/typed.rs b/helix-term/src/commands/typed.rs
index c7d7a757..1c59340c 100644
--- a/helix-term/src/commands/typed.rs
+++ b/helix-term/src/commands/typed.rs
@@ -375,7 +375,7 @@ fn write_impl(
 
 fn insert_final_newline(doc: &mut Document, view_id: ViewId) {
     let text = doc.text();
-    if line_ending::get_line_ending(&text.slice(..)).is_none() {
+    if text.len_chars() > 0 && line_ending::get_line_ending(&text.slice(..)).is_none() {
         let eof = Selection::point(text.len_chars());
         let insert = Transaction::insert(text, &eof, doc.line_ending.as_str().into());
         doc.apply(&insert, view_id);

the fix is pretty simple and straightforward ↑

@verte-zerg
Copy link

@Axlefublr Would you like to prepare PR with these changes?

@Axlefublr
Copy link
Contributor Author

@Axlefublr Would you like to prepare PR with these changes?

only if I get some certainty it will get accepted. I despise keeping a PR branch up to date only for it to be ignored for 3 years and then get rejected

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-command Area: Commands
Projects
None yet
Development

No branches or pull requests

3 participants