Skip to content

/ servo

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

Jump to bottom

editor modelines should be a tidy error #10719

Closed
edunham opened this issue Apr 19, 2016 · 2 comments
Closed

editor modelines should be a tidy error #10719

edunham opened this issue Apr 19, 2016 · 2 comments
Labels
A-infrastructure E-easy L-python

Comments

@edunham
Copy link
Contributor

@edunham edunham commented Apr 19, 2016 •
edited

Somewhere in https://github.com/servo/servo/blob/master/python/tidy/servo_tidy/tidy.py we should check for editor detritus like

-*- mode: modename; var: value; … -*-
# vim: set expandtab:

see https://www.gnu.org/software/emacs/manual/html_node/emacs/Specifying-File-Variables.html#Specifying-File-Variables, http://vim.wikia.com/wiki/Modeline_magic , and take a look through servo for any other editors' leftovers to catch if they exist.

As of #10723, we actually want to keep the modelines in /tests/wpt. Naive modeline checking is present but disabled in 93c3a6e; it needs to be extended to ignore checks within a specific directory just like other functions in the script.

Note that the PR to fix this will need to include or follow removal of modelines from the rest of the project.
@edunham
Copy link
Contributor Author

@edunham edunham commented Apr 19, 2016 

13:23:30 < aneeshusa> right, I forgot about those, I've never been a fan of editor modelines in code
13:23:36 <@edunham> larsberg: speaking of which, should editor residue lint as an error?
13:23:50 < aneeshusa> +1 yes please
13:24:11 <@larsberg> yeah, I think that makes sense, too
edunham added a commit to edunham/servo that referenced this issue Apr 19, 2016
@edunham
Improve tidy's license validation logic
93a8b9b 
fixes servo#10716

I took the lazy way out and hardcoded the size of block we examine for
licenses.

fixes servo#10719

Includes tests for new functionality.
edunham added a commit to edunham/servo that referenced this issue Apr 19, 2016
@edunham
Improve tidy's license validation logic
5a0c1cf 
fixes servo#10716

I took the lazy way out and hardcoded the size of block we examine for
licenses.

fixes servo#10719

Includes tests for new functionality.
edunham added a commit to edunham/servo that referenced this issue Apr 19, 2016
@edunham
Improve tidy's license validation logic
7104dd4 
fixes servo#10716

I took the lazy way out and hardcoded the size of block we examine for
licenses.

gets us set to fix servo#10719 by uncommenting
a few lines

Includes tests for new functionality.
edunham added a commit to edunham/servo that referenced this issue Apr 19, 2016
@edunham
Improve tidy's license validation logic
c1f777b 
fixes servo#10716

I took the lazy way out and hardcoded the size of block we examine for
licenses.

gets us set to fix servo#10719 by uncommenting
a few lines

Includes tests for new functionality.
edunham added a commit to edunham/servo that referenced this issue Apr 19, 2016
@edunham
Improve tidy's license validation logic
bde3777 
fixes servo#10716

I took the lazy way out and hardcoded the size of block we examine for
licenses.

fixes servo#10719

Includes tests for new functionality.

Conflicts:
	python/tidy/servo_tidy/licenseck.py
	python/tidy/setup.py
edunham added a commit to edunham/servo that referenced this issue Apr 19, 2016
@edunham
Improve tidy's license validation logic
1a6c98f 
fixes servo#10716

I took the lazy way out and hardcoded the size of block we examine for
licenses.

fixes servo#10719

Includes tests for new functionality.

Conflicts:
	python/tidy/servo_tidy/licenseck.py
	python/tidy/setup.py
@edunham edunham mentioned this issue Apr 19, 2016
Closed
edunham added a commit to edunham/servo that referenced this issue Apr 19, 2016
@edunham
Improve tidy's license validation logic
bf9444c 
fixes servo#10716

I took the lazy way out and hardcoded the size of block we examine for
licenses.

fixes servo#10719

Includes tests for new functionality.

Conflicts:
	python/tidy/servo_tidy/licenseck.py
	python/tidy/setup.py
edunham added a commit to edunham/servo that referenced this issue Apr 19, 2016
@edunham
Improve tidy's license validation logic
93c3a6e 
fixes servo#10716

I took the lazy way out and hardcoded the size of block we examine for
licenses.

fixes servo#10719

Includes tests for new functionality.

Conflicts:
	python/tidy/servo_tidy/licenseck.py
	python/tidy/setup.py
@edunham edunham mentioned this issue Apr 19, 2016
Merged
@zwn zwn mentioned this issue Apr 21, 2016
Merged
bors-servo added a commit that referenced this issue Apr 22, 2016
@bors-servo
Auto merge of #10723 - edunham:modelines, r=Wafflespeanut
Loading status checks…
dff217c 
Remove some Emacs & Vim modelines

This might be a bad idea, especially on the webidl side. However, we started talking about the idea that modelines are a lint error (#10719), and these changes would be required before enabling a modeline lint.

If it bitrots, it's easy to recreate with
```
find * -type f -exec sed '/- Mode:/d' -i {} +
find * -type f -exec sed '/ vim:/d' -i {} +
git checkout -- python/tidy/servo_tidy/tidy.py
git checkout -- python/tidy/servo_tidy_tests/spec.webidl
git commit -a
```

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/10723)
<!-- Reviewable:end -->
bors-servo added a commit that referenced this issue Apr 23, 2016
@bors-servo
Auto merge of #10786 - zwn:tidy-modelines, r=Manishearth
Loading status checks…
558bef7 
tidy check for vim and emacs modelines

See #10719.

It skips *.webidl files for now since I am not sure where they come from and if they should be edited in tree or not.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/10786)
<!-- Reviewable:end -->
bors-servo added a commit that referenced this issue Apr 23, 2016
@bors-servo
Auto merge of #10786 - zwn:tidy-modelines, r=Manishearth
Loading status checks…
04f8ae5 
tidy check for vim and emacs modelines

See #10719.

It skips *.webidl files for now since I am not sure where they come from and if they should be edited in tree or not.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/10786)
<!-- Reviewable:end -->
@wafflespeanut
Copy link
Member

@wafflespeanut wafflespeanut commented May 10, 2016

fixed by #10723, #10786
UK992 added a commit to UK992/servo that referenced this issue Aug 9, 2016
@edunham @UK992
Improve tidy's license validation logic
8e2b456 
fixes servo#10716

I took the lazy way out and hardcoded the size of block we examine for
licenses.

fixes servo#10719

Includes tests for new functionality.

Conflicts:
	python/tidy/servo_tidy/licenseck.py
	python/tidy/setup.py
UK992 added a commit to UK992/servo that referenced this issue Aug 9, 2016
@edunham @UK992
Improve tidy's license validation logic
f5eaefd 
fixes servo#10716

I took the lazy way out and hardcoded the size of block we examine for
licenses.

fixes servo#10719

Includes tests for new functionality.

Conflicts:
	python/tidy/servo_tidy/licenseck.py
	python/tidy/setup.py
UK992 added a commit to UK992/servo that referenced this issue Aug 9, 2016
@edunham @UK992
Improve tidy's license validation logic
4521d53 
fixes servo#10716

I took the lazy way out and hardcoded the size of block we examine for
licenses.

fixes servo#10719

Includes tests for new functionality.
UK992 added a commit to UK992/servo that referenced this issue Aug 9, 2016
@edunham @UK992
Improve tidy's license validation logic
bd2bf06 
fixes servo#10716

I took the lazy way out and hardcoded the size of block we examine for
licenses.

fixes servo#10719

Includes tests for new functionality.
UK992 added a commit to UK992/servo that referenced this issue Aug 9, 2016
@edunham @UK992
Improve tidy's license validation logic
664834e 
fixes servo#10716

I took the lazy way out and hardcoded the size of block we examine for
licenses.

fixes servo#10719

Includes tests for new functionality.
UK992 added a commit to UK992/servo that referenced this issue Aug 12, 2016
@edunham @UK992
Improve tidy's license validation logic
48ace17 
fixes servo#10716

I took the lazy way out and hardcoded the size of block we examine for
licenses.

fixes servo#10719

Includes tests for new functionality.
samuknet added a commit to samuknet/servo that referenced this issue Sep 6, 2016
@edunham @samuknet
Improve tidy's license validation logic
f033d3b 
fixes servo#10716

I took the lazy way out and hardcoded the size of block we examine for
licenses.

fixes servo#10719

Includes tests for new functionality.
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this issue Oct 1, 2019
@marco-c
servo: Merge #10723 - Remove some Emacs & Vim modelines (from edunham…
90f8b1f 
…:modelines); r=Wafflespeanut

This might be a bad idea, especially on the webidl side. However, we started talking about the idea that modelines are a lint error (servo/servo#10719), and these changes would be required before enabling a modeline lint.

If it bitrots, it's easy to recreate with
```
find * -type f -exec sed '/- Mode:/d' -i {} +
find * -type f -exec sed '/ vim:/d' -i {} +
git checkout -- python/tidy/servo_tidy/tidy.py
git checkout -- python/tidy/servo_tidy_tests/spec.webidl
git commit -a
```

Source-Repo: https://github.com/servo/servo
Source-Revision: dff217c2e3ff0b77eeebf62d36c2bf57c044cf14

UltraBlame original commit: afc9690e74e56a8b818fd238fe4e85bde0e75a65
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this issue Oct 1, 2019
@marco-c
servo: Merge #10723 - Remove some Emacs & Vim modelines (from edunham…
786ccdb 
…:modelines); r=Wafflespeanut

This might be a bad idea, especially on the webidl side. However, we started talking about the idea that modelines are a lint error (servo/servo#10719), and these changes would be required before enabling a modeline lint.

If it bitrots, it's easy to recreate with
```
find * -type f -exec sed '/- Mode:/d' -i {} +
find * -type f -exec sed '/ vim:/d' -i {} +
git checkout -- python/tidy/servo_tidy/tidy.py
git checkout -- python/tidy/servo_tidy_tests/spec.webidl
git commit -a
```

Source-Repo: https://github.com/servo/servo
Source-Revision: dff217c2e3ff0b77eeebf62d36c2bf57c044cf14

UltraBlame original commit: afc9690e74e56a8b818fd238fe4e85bde0e75a65
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this issue Oct 1, 2019
@marco-c
servo: Merge #10723 - Remove some Emacs & Vim modelines (from edunham…
7f3761c 
…:modelines); r=Wafflespeanut

This might be a bad idea, especially on the webidl side. However, we started talking about the idea that modelines are a lint error (servo/servo#10719), and these changes would be required before enabling a modeline lint.

If it bitrots, it's easy to recreate with
```
find * -type f -exec sed '/- Mode:/d' -i {} +
find * -type f -exec sed '/ vim:/d' -i {} +
git checkout -- python/tidy/servo_tidy/tidy.py
git checkout -- python/tidy/servo_tidy_tests/spec.webidl
git commit -a
```

Source-Repo: https://github.com/servo/servo
Source-Revision: dff217c2e3ff0b77eeebf62d36c2bf57c044cf14

UltraBlame original commit: afc9690e74e56a8b818fd238fe4e85bde0e75a65
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-infrastructure E-easy L-python
Projects
None yet
Milestone
No milestone
Linked pull requests

Successfully merging a pull request may close this issue.

Improve tidy's license validation logic
2 participants
@edunham @wafflespeanut
You can’t perform that action at this time.