Make whitespace / newline linter also fail for extra empty lines at the end of files (\n\n$ => \n$)
Categories
(Developer Infrastructure :: Lint and Formatting, task, P3)
Tracking
(firefox86 fixed)
Tracking | Status | |
---|---|---|
firefox86 | --- | fixed |
People
(Reporter: Gijs, Assigned: ikartikgautam, Mentored)
References
Details
(Keywords: good-first-bug, Whiteboard: [lang=python])
Attachments
(1 file)
As in summary. This is catching people out for l10n files, and it should really be a standard for all our other code files, too.
Comment 1•4 years ago
|
||
The severity field is not set for this bug.
:ahal, could you have a look please?
For more information, please visit auto_nag documentation.
Updated•4 years ago
|
Comment 2•4 years ago
|
||
Seems like a great good first bug.
Should be added here:
https://searchfox.org/mozilla-central/source/tools/lint/file-whitespace/__init__.py#15
doc is here:
https://firefox-source-docs.mozilla.org/code-quality/lint/linters/file-whitespace.html
Tests:
https://searchfox.org/mozilla-central/source/tools/lint/test/test_file_whitespace.py
Assignee | ||
Comment 3•4 years ago
|
||
Hi,
I am very new to contributing to mozilla. Sir can you please guide me on how should i approach this issue and fix it. I think i can fix this issue. I just need your help. I tried to figure out myself fixing this issue, but unfortunately i failed, hence require your help.
Thank You,
Kartik Gautam
Comment 4•4 years ago
|
||
Sure, how can I help you? Please be specific in your request! :)
Cheers
Assignee | ||
Comment 5•4 years ago
|
||
Hi,
Sir, i am working on it, i just have one question am i required to only make linter fail for missing empty lines at the end of files or after failing it should also fix the error ?
Thank You,
Kartik Gautam
Comment 6•4 years ago
|
||
Good question.
First, you can do a first change which will detect this.
Then, you could indeed in the future fix it but it doesn't have to in the v1 :)
Assignee | ||
Comment 7•4 years ago
|
||
(In reply to Sylvestre Ledru [:Sylvestre] from comment #6)
Good question.
First, you can do a first change which will detect this.Then, you could indeed in the future fix it but it doesn't have to in the v1 :)
Sir, I have made the changes, Should i add you as reviewer? Also i am a little unsure about the error messages, please take a look at them too.
Comment 8•4 years ago
|
||
Yeah, thanks :)
Assignee | ||
Comment 9•4 years ago
|
||
Depends on D97943
Updated•4 years ago
|
Assignee | ||
Comment 10•4 years ago
|
||
Sorry, i think i have done something wrong while submitting for review, i don’t know why it submitted my earlier contribution too. and here it commented "Depends on D97943". Can you please suggest me, what should i do ?
Comment 11•4 years ago
•
|
||
I removed the dependency for you (before seeing your message).
Updated•4 years ago
|
Comment 12•4 years ago
|
||
This patch and bug 1680556 are backward: the linter should fail if a file doesn't have an empty line at the end, not the other way around.
Assignee | ||
Comment 13•4 years ago
|
||
(In reply to Francesco Lodolo [:flod] from comment #12)
This patch and bug 1680556 are backward: the linter should fail if a file doesn't have an empty line at the end, not the other way around.
So does it mean that it should only give error if file does not have empty lines at end ? be it any number of of empty lines ?
Right now it checks for if file have any extra empty line at end of file.
Updated•4 years ago
|
Comment 14•4 years ago
|
||
Kartik, please continue to work on it. It is great :)
I replied in the reviews with details
Currently, you are working on:
----
mycode\n
\n
----
to be replaced by:
----
mycode\n
----
We will open a different bug for the second thing which is:
----
mycode
----
to be replaced by:
----
mycode\n
----
Comment 15•4 years ago
|
||
Comment 16•4 years ago
|
||
bugherder |
Updated•2 years ago
|
Description
•