Skip to content

Report removal of final newline #54

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

Merged
merged 1 commit into from
Jul 26, 2021
Merged

Report removal of final newline #54

merged 1 commit into from
Jul 26, 2021

Conversation

per1234
Copy link
Contributor

@per1234 per1234 commented Jul 24, 2021

If a file does not end in a newline, then every line added to the end of the file results in a diff to the previous final
line.

An example using \n to symbolize end of line characters:

https://github.com/arduino-libraries/Servo\n
https://github.com/arduino-libraries/Stepper

When another line is added at the end:

https://github.com/arduino-libraries/Servo\n
https://github.com/arduino-libraries/Stepper\n
https://github.com/arduino-libraries/FooBar

you can see there is necessarily a diff on line 2 even though the editor did not touch that line.

For this reason, it is best practices to always retain a newline at the end of files.

In addition to it being common practice, the GitHub web editor automatically adds this newline. Between this and my
expectation that people would either add URLs to the top of the list or in alphabetical order, leaving the distant end of
the file alone, I was hoping that this would not be a common problem with the library submission system, but this is
simply not a realistic expectation.

What I hadn’t considered is that the person to suffer for this formatting is the next to add a URL to the end of the
file. Because of this “ghost diff”, the submission system sees this PR as a modification to an existing item on the list
rather than an addition.

I think this will be best handled by detecting and dealing with the problem at its introduction, rather than attempting
to make the system resilient to a missing final newline.


The associated changes to the arduino/library-registry repository's "Manage PRs" workflow: per1234/library-registry@f8385ae
Demonstration of the updated parser in action: per1234/library-registry#8 (comment)

If a file does not end in a newline, then every line added to the end of the file results in a diff to the previous final
line.

An example using `\n` to symbolize end of line characters:

https://github.com/arduino-libraries/Servo\n
https://github.com/arduino-libraries/Stepper

When another line is added at the end:

https://github.com/arduino-libraries/Servo\n
https://github.com/arduino-libraries/Stepper\n
https://github.com/arduino-libraries/FooBar

you can see there is necessarily a diff on line 2 even though the editor did not touch that line.

For this reason, it is best practices to always retain a newline at the end of files.

In addition to it being common practice, the GitHub web editor automatically adds this newline. Between this and my
expectation that people would either add URLs to the top of the list or in alphabetical order, leaving the distant end of
the file alone, I was hoping that this would not be a common problem with the library submission system, but this is
simply not a realistic expectation.

What I hadn’t considered is that the person to suffer for this formatting is the next to add a URL to the end of the
file. Because of this “ghost diff”, the submission system sees this PR as a modification to an existing item on the list
rather than an addition.

I think this will be best handled by detecting and dealing with the problem at its introduction, rather than attempting
to make the system resilient to a missing final newline.
@per1234 per1234 added type: enhancement Proposed improvement topic: code Related to content of the project itself labels Jul 24, 2021
@per1234 per1234 requested review from silvanocerza and umbynos July 24, 2021 16:01
@per1234 per1234 merged commit a15ad39 into arduino:main Jul 26, 2021
@per1234 per1234 deleted the handle-final-newline-removal branch July 26, 2021 11:22
@per1234 per1234 self-assigned this Nov 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: code Related to content of the project itself type: enhancement Proposed improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants