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

insert_final_newline == false --- strip EOL, or leave untouched? #475

Open
cxw42 opened this issue Oct 30, 2022 · 14 comments
Open

insert_final_newline == false --- strip EOL, or leave untouched? #475

cxw42 opened this issue Oct 30, 2022 · 14 comments

Comments

@cxw42
Copy link
Member

cxw42 commented Oct 30, 2022

The specification says (emphasis added):

insert_final_newline: Set to true ensure file ends with a newline when saving and false to ensure it doesn’t.

However:

  • the Vim plugin, back in 2014, treated false as "leave untouched", not "remove trailing newline".
  • @treyhunner said in 2012 that "If insert_final_newline = false the text editor is simply instructed to not auto-insert final newlines".

Which is it? Does insert_final_newline == false mean:

  1. Don't touch the last EOL (as implemented); or
  2. Strip the last EOL (as specified)

?

I think it's option 1, and that the specification should be updated, but I'm not sure. Thanks!

Motivating example: editorconfig/editorconfig-vim#136

Relates to #59 and #151

@xuhdev
Copy link
Member

xuhdev commented Oct 31, 2022

I remember that the Vim plugin implementation should be correct. However, editors have limitations and enforcing either should lead to some editors not being able to do so.

@ffes
Copy link
Member

ffes commented Oct 31, 2022

You are right, @cxw42. Clarifying this text a bit more in the specs could be useful. But as @xuhdev says, editors have their limitations.

FYI, the Notepad++ plugin deletes the final newline(s) when insert_final_newline = false.

@cxw42
Copy link
Member Author

cxw42 commented Oct 31, 2022

I agree not all editors can implement all properties. However, I still think we should at least make sure the specification lists the intended behaviour of the property, so that plugin authors know the goal.

krobelus added a commit to krobelus/kakoune that referenced this issue Nov 19, 2022
For simplicity of implementation, each Kakoune buffer has at least one
line, and each line has at least one character, the trailing newline
("\n").

When writing a buffer to disk, we include the trailing newlines,
independent of whether they are present in the underpinning file.

There are some [use-cases] where this is undesirable, like when editing
a MOTD file or `<pre>`-formatted text on a web page.  Additionally,
there are plenty of existing [source code files] that are missing
a trailing newline (probably because Emacs creates such files).

I've been using this workaround to avoid unintentional changes:

	hook -always global BufOpenFile .* %{ evaluate-commands %sh{
		if [ $(tail -c1 "$kak_hook_param" | wc -l) = 0 ]; then {
			echo 'hook -always buffer BufWritePost .* %{
				nop %sh{ truncate --size=-1 "$kak_hook_param" }
			}'
		} fi
	} }

The -always switches are necessary when using [find-apply-changes]
which edits and writes files in a draft context with "-no-hooks"
(probably to suppress auto-format hooks).

Obviate the need for this workaround by making Kakoune preserve
missing newlines in the input.
Special care is taken of empty files: they are written back as empty
(without a newline) but if they are no longer empty we do include
the newline.

This patch can be thought of as "damage control".  I believe it does
not make any real case worse because proper termination of files
should already be done at creation time.  We shouldn't try to fix
the user's mistake here. Other costs are
1. Implementation complexity - but it's pretty contained.
2. We are giving the user the pretense that the file has a newline
   even if that won't be written to disk.

The patch makes us match GNU sed but deviate from Vim (which adds
missing newlines).

PR mawww#3724 suggested making this behavior configurable.  We could do
that with either approach.  However, independent of the approach,
this would add some complexity: when reloading a buffer from disk we'd
want to distinguish between explicitly-set and inferred EOL-at-EOF
option to avoid stomping user configuration.
I also think that the configurability is of limited use.
There is even an EditorConfig option but it does not include the
sensible behavior of preserving input, see the [editorconfig-issue].

Closes mawww#2147
Closes mawww#3724

[use-cases]: mawww#2147 (comment)
[source code files]: mawww#2147 (comment)
[find-apply-changes]: https://github.com/occivink/kakoune-find/
[editorconfig-issue]: editorconfig/editorconfig#475
krobelus added a commit to krobelus/kakoune that referenced this issue Nov 19, 2022
For simplicity of implementation, each Kakoune buffer has at least one
line, and each line has at least one character, the trailing newline
("\n").

When writing a buffer to disk, we include the trailing newlines,
independent of whether they are present in the underpinning file.

There are some [use-cases] where this is undesirable, like when editing
a MOTD file or `<pre>`-formatted text on a web page.  Additionally,
there are plenty of existing [source code files] that are missing
a trailing newline (probably because Emacs creates such files).

I've been using this workaround to avoid unintentional changes:

	hook -always global BufOpenFile .* %{ evaluate-commands %sh{
		if [ $(tail -c1 "$kak_hook_param" | wc -l) = 0 ]; then {
			echo 'hook -always buffer BufWritePost .* %{
				nop %sh{ truncate --size=-1 "$kak_hook_param" }
			}'
		} fi
	} }

The -always switches are necessary when using [find-apply-changes]
which edits and writes files in a draft context with "-no-hooks"
(probably to suppress auto-format hooks).

Obviate the need for this workaround by making Kakoune preserve
missing newlines in the input.
Special care is taken of empty files: they are written back as empty
(without a newline) but if they are no longer empty we do include
the newline.

This patch can be thought of as "damage control".  I believe it does
not make any real case worse because proper termination of files
should already be done at creation time.  We shouldn't try to fix
the user's mistake here. Other costs are
1. Implementation complexity - but it's pretty contained.
2. We are giving the user the pretense that the file has a newline
   even if that won't be written to disk.

The patch makes us match GNU sed but deviate from Vim (which adds
missing newlines).

PR mawww#3724 suggested making this behavior configurable.  We could do
that with either approach.  However, independent of the approach,
this would add some complexity: when reloading a buffer from disk we'd
want to distinguish between explicitly-set and inferred EOL-at-EOF
option to avoid stomping user configuration.
I also think that the configurability is of limited use.
There is even an EditorConfig option but it does not include the
sensible behavior of preserving input, see the [editorconfig-issue].
Overall, I don't think we should make this configurable unless
there is a strong reason. It should be easy to add workarounds
to manually fix up line endings.

Closes mawww#2147
Closes mawww#3724

[use-cases]: mawww#2147 (comment)
[source code files]: mawww#2147 (comment)
[find-apply-changes]: https://github.com/occivink/kakoune-find/
[editorconfig-issue]: editorconfig/editorconfig#475
krobelus added a commit to krobelus/kakoune that referenced this issue Nov 20, 2022
For simplicity of implementation, each Kakoune buffer has at least one
line, and each line has at least one character, the trailing newline
("\n").

When writing a buffer to disk, we include the trailing newlines,
independent of whether they are present in the underpinning file.

There are some [use-cases] where this is undesirable, like when editing
a MOTD file or `<pre>`-formatted text on a web page.  Additionally,
there are plenty of existing [source code files] that are missing
a trailing newline (probably because Emacs creates such files).

I've been using this workaround to avoid unintentional changes:

	hook -always global BufOpenFile .* %{ evaluate-commands %sh{
		if [ $(tail -c1 "$kak_hook_param" | wc -l) = 0 ]; then {
			echo 'hook -always buffer BufWritePost .* %{
				nop %sh{ truncate --size=-1 "$kak_hook_param" }
			}'
		} fi
	} }

The -always switches are necessary when using [find-apply-changes]
which edits and writes files in a draft context with "-no-hooks"
(probably to suppress auto-format hooks).

Obviate the need for this workaround by making Kakoune preserve
missing newlines in the input.
Special care is taken of empty files: they are written back as empty
(without a newline) but if they are no longer empty we do include
the newline.

This patch can be thought of as "damage control".  I believe it does
not make any real case worse because proper termination of files
should already be done at creation time.  We shouldn't try to fix
the user's mistake here. Other costs are
1. Implementation complexity - but it's pretty contained.
2. We are giving the user the pretense that the file has a newline
   even if that won't be written to disk.

The patch makes us match GNU sed but deviate from Vim (which adds
missing newlines).

PR mawww#3724 suggested making this behavior configurable.  We could do
that with either approach.  However, independent of the approach,
this would add some complexity: when reloading a buffer from disk we'd
want to distinguish between explicitly-set and inferred EOL-at-EOF
option to avoid stomping user configuration.
I also think that the configurability is of limited use.
There is even an EditorConfig option but it does not include the
sensible behavior of preserving input, see the [editorconfig-issue].
Overall, I don't think we should make this configurable unless
there is a strong reason. It should be easy to add workarounds
to manually fix up line endings.

Closes mawww#2147
Closes mawww#3724

[use-cases]: mawww#2147 (comment)
[source code files]: mawww#2147 (comment)
[find-apply-changes]: https://github.com/occivink/kakoune-find/
[editorconfig-issue]: editorconfig/editorconfig#475
@renzedj
Copy link

renzedj commented Mar 2, 2023

So, to validate: The intended behavior of insert_final_newline = true is:

  • true: Ensure that the file ends with a newline.
  • false: Strip the final newline(s) from the file.

The third use case is: Multiple newlines at the end of the file (e.g., \n\n\n). If true, is the intent of the specification for insert_final_newline to strip newlines and leave the file with one and only one newline at the end of the file (i.e., \n), or does it leave it untouched? I would think from the both the naming schema and specification that it should leave it untouched.

However, when I requested a feature in #483 to either trim multiple newlines at the end of a file (leaving one and only one newline) or specify the number of blank lines/newlines to leave at the end of the file, I was told that this issue indicates that the specification says that insert_final_newline = true means that there should be one and only one newline at the end of a file. I don't read it that way, so perhaps I was unclear in what I was requesting.

Can someone please provide clarification?

Thx.

@xuhdev
Copy link
Member

xuhdev commented Mar 4, 2023

I don't think they are the same. However, I don't think we'll implement exactly N new lines at EOF because this is not a feature that's widely available among editors (because, this feature doesn't have many use cases).

However, this issue should still be relevant to what you are looking for (exactly 1 EOL or only must end with EOL?)

@renzedj
Copy link

renzedj commented Mar 6, 2023

I don't think they are the same. However, I don't think we'll implement exactly N new lines at EOF because this is not a feature that's widely available among editors (because, this feature doesn't have many use cases).

However, this issue should still be relevant to what you are looking for (exactly 1 EOL or only must end with EOL?)

Thx. I didn't figure my feature request would be accepted, but at this point I'm simply looking for clarification regarding the intent of the specification. If the intent is to end with one-and-only-one EOL, then I'll go back to the author of the plugin, otherwise I'd request that this be added to the specification for this feature, as I think it's implied.

justinmk added a commit to justinmk/neovim that referenced this issue Mar 9, 2023
Problem:
Nvim editorconfig support actively removes a newline if
`insert_final_newline=false`. This (apparently) contradicts the spec.
editorconfig/editorconfig#475

Solution:
Don't set options if `insert_final_newline=false`.

fix neovim#21648
@cxw42
Copy link
Member Author

cxw42 commented Mar 13, 2023

I think I would be surprised if a property called "insert_final_newline" caused newlines to be removed :) . I believe we should update the specification to say that insert_final_newline = false means don't touch the newlines at EOF. I suggest:

 insert_final_newline: Set to true
+to
  ensure file ends with a newline when 
-saving
+saving.
-and false to ensure it doesn’t.
+If it is set to false, the newline(s) at the end of the file will neither be checked nor modified.

What say you all?

@xuhdev
Copy link
Member

xuhdev commented Mar 13, 2023

Actually I don't think this is true. If the existing file has only 1 final newline, some editors will delete this final newline.

What if the file has multiple newlines and the plugin tells the editor to not add final new lines? I guess editors will do this in wildly different ways (not verified).

@justinmk
Copy link

justinmk commented Mar 13, 2023

If the existing file has only 1 final newline, some editors will delete this final newline.

Editors do all kinds of things, but why is that relevant to editorconfig ?

The name insert_final_newline implies a decision to force, or not force, a final newline. insert_final_newline=false reads as "don't force \n on the last line", it doesn't read as "forcibly remove \n from the last line". That is why the current spec is confusing, because it says false to ensure it doesn’t which directly contradicts the wording of the option itself (which is what users notice).

The central problem is

  1. the option name directly contradicts the behavior described by the spec. The option name insert_final_newline is what users see, they don't read the spec.
  2. the "official" editorconfig-vim plugin does not implement the (current) spec: https://github.com/editorconfig/editorconfig-vim/blob/5b875ac1aeba22abce3c499c0d5e4f33558fdf2c/plugin/editorconfig.vim#L471-L472

krobelus added a commit to krobelus/kakoune that referenced this issue Apr 16, 2023
For simplicity of implementation, each Kakoune buffer has at least one
line, and each line has at least one character, the trailing newline
("\n").

When writing a buffer to disk, we include the trailing newlines,
independent of whether they are present in the underpinning file.

There are some [use-cases] where this is undesirable, like when editing
a MOTD file or `<pre>`-formatted text on a web page.  Additionally,
there are plenty of existing [source code files] that are missing
a trailing newline (probably because Emacs creates such files).

I've been using this workaround to avoid unintentional changes:

	hook -always global BufOpenFile .* %{ evaluate-commands %sh{
		if [ $(tail -c1 "$kak_hook_param" | wc -l) = 0 ]; then {
			echo 'hook -always buffer BufWritePost .* %{
				nop %sh{ truncate --size=-1 "$kak_hook_param" }
			}'
		} fi
	} }

The -always switches are necessary when using [find-apply-changes]
which edits and writes files in a draft context with "-no-hooks"
(probably to suppress auto-format hooks).

Obviate the need for this workaround by making Kakoune preserve
missing newlines in the input.
Special care is taken of empty files: they are written back as empty
(without a newline) but if they are no longer empty we do include
the newline.

This patch can be thought of as "damage control".  I believe it does
not make any real case worse because proper termination of files
should already be done at creation time.  We shouldn't try to fix
the user's mistake here. Other costs are
1. Implementation complexity - but it's pretty contained.
2. We are giving the user the pretense that the file has a newline
   even if that won't be written to disk.

The patch makes us match GNU sed but deviate from Vim (which adds
missing newlines).

PR mawww#3724 suggested making this behavior configurable.  We could do
that with either approach.  However, independent of the approach,
this would add some complexity: when reloading a buffer from disk we'd
want to distinguish between explicitly-set and inferred EOL-at-EOF
option to avoid stomping user configuration.
I also think that the configurability is of limited use.
There is even an EditorConfig option but it does not include the
sensible behavior of preserving input, see the [editorconfig-issue].
Overall, I don't think we should make this configurable unless
there is a strong reason. It should be easy to add workarounds
to manually fix up line endings.

Closes mawww#2147
Closes mawww#3724

[use-cases]: mawww#2147 (comment)
[source code files]: mawww#2147 (comment)
[find-apply-changes]: https://github.com/occivink/kakoune-find/
[editorconfig-issue]: editorconfig/editorconfig#475
krobelus added a commit to krobelus/kakoune that referenced this issue Apr 24, 2023
For simplicity of implementation, each Kakoune buffer has at least one
line, and each line has at least one character, the trailing newline
("\n").

When writing a buffer to disk, we include the trailing newlines,
independent of whether they are present in the underpinning file.

There are some [use-cases] where this is undesirable, like when editing
a MOTD file or `<pre>`-formatted text on a web page.  Additionally,
there are plenty of existing [source code files] that are missing
a trailing newline (probably because Emacs creates such files).

I've been using this workaround to avoid unintentional changes:

	hook -always global BufOpenFile .* %{ evaluate-commands %sh{
		if [ $(tail -c1 "$kak_hook_param" | wc -l) = 0 ]; then {
			echo 'hook -always buffer BufWritePost .* %{
				nop %sh{ truncate --size=-1 "$kak_hook_param" }
			}'
		} fi
	} }

The -always switches are necessary when using [find-apply-changes]
which edits and writes files in a draft context with "-no-hooks"
(probably to suppress auto-format hooks).

Obviate the need for this workaround by making Kakoune preserve
missing newlines in the input.
Special care is taken of empty files: they are written back as empty
(without a newline) but if they are no longer empty we do include
the newline.

This patch can be thought of as "damage control".  I believe it does
not make any real case worse because proper termination of files
should already be done at creation time.  We shouldn't try to fix
the user's mistake here. Other costs are
1. Implementation complexity - but it's pretty contained.
2. We are giving the user the pretense that the file has a newline
   even if that won't be written to disk.

The patch makes us match GNU sed but deviate from Vim (which adds
missing newlines).

PR mawww#3724 suggested making this behavior configurable.  We could do
that with either approach.  However, independent of the approach,
this would add some complexity: when reloading a buffer from disk we'd
want to distinguish between explicitly-set and inferred EOL-at-EOF
option to avoid stomping user configuration.
I also think that the configurability is of limited use.
There is even an EditorConfig option but it does not include the
sensible behavior of preserving input, see the [editorconfig-issue].
Overall, I don't think we should make this configurable unless
there is a strong reason. It should be easy to add workarounds
to manually fix up line endings.

Closes mawww#2147
Closes mawww#3724

[use-cases]: mawww#2147 (comment)
[source code files]: mawww#2147 (comment)
[find-apply-changes]: https://github.com/occivink/kakoune-find/
[editorconfig-issue]: editorconfig/editorconfig#475
krobelus added a commit to krobelus/kakoune that referenced this issue May 19, 2023
For simplicity of implementation, each Kakoune buffer has at least one
line, and each line has at least one character, the trailing newline
("\n").

When writing a buffer to disk, we include the trailing newlines,
independent of whether they are present in the underpinning file.

There are some [use-cases] where this is undesirable, like when editing
a MOTD file or `<pre>`-formatted text on a web page.  Additionally,
there are plenty of existing [source code files] that are missing
a trailing newline (probably because Emacs creates such files).

I've been using this workaround to avoid unintentional changes:

	hook -always global BufOpenFile .* %{ evaluate-commands %sh{
		if [ $(tail -c1 "$kak_hook_param" | wc -l) = 0 ]; then {
			echo 'hook -always buffer BufWritePost .* %{
				nop %sh{ truncate --size=-1 "$kak_hook_param" }
			}'
		} fi
	} }

The -always switches are necessary when using [find-apply-changes]
which edits and writes files in a draft context with "-no-hooks"
(probably to suppress auto-format hooks).

Obviate the need for this workaround by making Kakoune preserve
missing newlines in the input.
Special care is taken of empty files: they are written back as empty
(without a newline) but if they are no longer empty we do include
the newline.

This patch can be thought of as "damage control".  I believe it does
not make any real case worse because proper termination of files
should already be done at creation time.  We shouldn't try to fix
the user's mistake here. Other costs are
1. Implementation complexity - but it's pretty contained.
2. We are giving the user the pretense that the file has a newline
   even if that won't be written to disk.

The patch makes us match GNU sed but deviate from Vim (which adds
missing newlines).

PR mawww#3724 suggested making this behavior configurable.  We could do
that with either approach.  However, independent of the approach,
this would add some complexity: when reloading a buffer from disk we'd
want to distinguish between explicitly-set and inferred EOL-at-EOF
option to avoid stomping user configuration.
I also think that the configurability is of limited use.
There is even an EditorConfig option but it does not include the
sensible behavior of preserving input, see the [editorconfig-issue].
Overall, I don't think we should make this configurable unless
there is a strong reason. It should be easy to add workarounds
to manually fix up line endings.

Closes mawww#2147
Closes mawww#3724

[use-cases]: mawww#2147 (comment)
[source code files]: mawww#2147 (comment)
[find-apply-changes]: https://github.com/occivink/kakoune-find/
[editorconfig-issue]: editorconfig/editorconfig#475
@xuhdev
Copy link
Member

xuhdev commented Jun 2, 2023

Yes, this is a bit confusing. Vim plugin doesn't do it partly because there's no way (or no clean way) to enforce no EOF of a file. If memory serves, we made the compromise in Vim but thought insert_final_newline=false should normally imply forcing no EOF.

So what has really caused the confusion here may be that the spec should explicitly state that a plugin doesn't have to implement each single requirement -- they need to do their best in the context of the working editor. This is kind of obvious for developers in this circle but may not be obvious for anyone new to this system. What do you think?

@renzedj
Copy link

renzedj commented Jun 2, 2023

So what has really caused the confusion here may be that the spec should explicitly state that a plugin doesn't have to implement each single requirement -- they need to do their best in the context of the working editor. This is kind of obvious for developers in this circle but may not be obvious for anyone new to this system. What do you think?

I would also add the requirement that where there is a difference between the specification and the behavior of the plugin, that difference MUST be documented. That would help reduce noise.

@gpanders
Copy link

gpanders commented Aug 9, 2023

To second what @justinmk said, I would like to propose that the EditorConfig specification be amended in one of two ways:

  1. Clarify and enforce
    • The wording of the specification remains the same, but the current option (insert_final_newline) is deprecated and replaced with something like enforce_final_newline=[insert|remove] or similar
  2. Relax
    • Modify the text in the specification to read "Set to true ensure file ends with a newline". No other text is needed: this is a boolean option, so if it's not set to true then the obvious implication is that it simply doesn't do anything and is as good as not setting the option at all

Taking the position that "editors should just implement the spec for whatever makes sense to them" diminishes the value of having a specification at all, and we will very soon (if we haven't already) run into "well it works in my editor" kind of issues. The whole point of having EditorConfig is to have common behavior enforced across editors. This ambiguity in the specification is detrimental to that objective.

@lewis6991
Copy link

The third use case is: Multiple newlines at the end of the file (e.g., \n\n\n). If true, is the intent of the specification for insert_final_newline to strip newlines

Given this situation, I think it's pretty clear that insert_final_newline==false shouldn't mean "strip any trailing newlines from the file". The wording of the spec needs to be clarified and relaxed.

@cxw42
Copy link
Member Author

cxw42 commented Nov 6, 2023

I agree with

  1. Relax
    • Modify the text in the specification to read "Set to true ensure file ends with a newline"

krobelus added a commit to krobelus/kakoune that referenced this issue Nov 19, 2023
For simplicity of implementation, each Kakoune buffer has at least one
line, and each line has at least one character, the trailing newline
("\n").

When writing a buffer to disk, we include the trailing newlines,
independent of whether they are present in the underpinning file.

There are some [use-cases] where this is undesirable, like when editing
a MOTD file or `<pre>`-formatted text on a web page.  Additionally,
there are plenty of existing [source code files] that are missing
a trailing newline (probably because Emacs creates such files).

I've been using this workaround to avoid unintentional changes:

	hook -always global BufOpenFile .* %{ evaluate-commands %sh{
		if [ $(tail -c1 "$kak_hook_param" | wc -l) = 0 ]; then {
			echo 'hook -always buffer BufWritePost .* %{
				nop %sh{ truncate --size=-1 "$kak_hook_param" }
			}'
		} fi
	} }

The -always switches are necessary when using [find-apply-changes]
which edits and writes files in a draft context with "-no-hooks"
(probably to suppress auto-format hooks).

Obviate the need for this workaround by making Kakoune preserve
missing newlines in the input.
Special care is taken of empty files: they are written back as empty
(without a newline) but if they are no longer empty we do include
the newline.

This patch can be thought of as "damage control".  I believe it does
not make any real case worse because proper termination of files
should already be done at creation time.  We shouldn't try to fix
the user's mistake here. Other costs are
1. Implementation complexity - but it's pretty contained.
2. We are giving the user the pretense that the file has a newline
   even if that won't be written to disk.

The patch makes us match GNU sed but deviate from Vim (which adds
missing newlines).

PR mawww#3724 suggested making this behavior configurable.  We could do
that with either approach.  However, independent of the approach,
this would add some complexity: when reloading a buffer from disk we'd
want to distinguish between explicitly-set and inferred EOL-at-EOF
option to avoid stomping user configuration.
I also think that the configurability is of limited use.
There is even an EditorConfig option but it does not include the
sensible behavior of preserving input, see the [editorconfig-issue].
Overall, I don't think we should make this configurable unless
there is a strong reason. It should be easy to add workarounds
to manually fix up line endings.

Closes mawww#2147
Closes mawww#3724

[use-cases]: mawww#2147 (comment)
[source code files]: mawww#2147 (comment)
[find-apply-changes]: https://github.com/occivink/kakoune-find/
[editorconfig-issue]: editorconfig/editorconfig#475
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants