Closed
Bug 1511383
Opened 5 years ago
Closed 5 years ago
Update vim modelines to wrap lines after 80 chars
Categories
(Developer Infrastructure :: Lint and Formatting, enhancement)
Developer Infrastructure
Lint and Formatting
Tracking
(firefox65 fixed)
RESOLVED
FIXED
mozilla65
Tracking | Status | |
---|---|---|
firefox65 | --- | fixed |
People
(Reporter: bbouvier, Assigned: bbouvier)
References
Details
Attachments
(1 file, 1 obsolete file)
861.88 KB,
patch
|
Sylvestre
:
review+
|
Details | Diff | Splinter Review |
No description provided.
Assignee | ||
Comment 1•5 years ago
|
||
Tried phabsend but it sent me a 500 error. This updates the vim modelines to wrap lines after 79 chars instead of 99 as in most files. Generated with the bash lines (for each of .h, .c and .cpp): for f in `ls *.h`; do sed -i -r "s/vim:(.*)tw=99(.*)/vim:\1tw=79\2/" $f; done
Assignee | ||
Comment 2•5 years ago
|
||
Comment on attachment 9028951 [details] [diff] [review] fix.patch Actually incorrect, it should be 80 chars, not 79 chars, after testing mach clang-format.
Attachment #9028951 -
Flags: review?(sledru)
Comment 3•5 years ago
|
||
I wonder if we should just remove those mode lines. They are often inconsistent with each other.
Comment 4•5 years ago
|
||
I'd like this to be discussed somewhere, but I don't know where that is... so starting here. I'm not sure why we are worrying about modelines. Do we really need them in all our code? If we do, then they're missing from an awful lot of code, and there would be a lot of maintenance or hooks or other things to ensure consistency. IMO we should be spending time to working on moving towards something like prettier, which will format the code and deal with line lengths in a better way.
Assignee | ||
Comment 5•5 years ago
|
||
That also includes changing the tab width to 2 spaces everywhere.
Attachment #9028951 -
Attachment is obsolete: true
Attachment #9028977 -
Flags: review?(sledru)
Assignee | ||
Updated•5 years ago
|
Summary: Update vim modelines to wrap lines after 79 chars → Update vim modelines to wrap lines after 80 chars
Comment 6•5 years ago
|
||
Comment on attachment 9028977 [details] [diff] [review] fix.patch Review of attachment 9028977 [details] [diff] [review]: ----------------------------------------------------------------- As an emacs user, Merci Benjamin! ;)
Attachment #9028977 -
Flags: review?(sledru) → review+
Pushed by bbouvier@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/e4712449ba43 Update vim modelines after clang-format; r=sylvestre
Comment 8•5 years ago
|
||
(In reply to Mark Banner (:standard8) from comment #4) > I'd like this to be discussed somewhere, but I don't know where that is... > so starting here. > > I'm not sure why we are worrying about modelines. Do we really need them in > all our code? If we do, then they're missing from an awful lot of code, and > there would be a lot of maintenance or hooks or other things to ensure > consistency. > > IMO we should be spending time to working on moving towards something like > prettier, which will format the code and deal with line lengths in a better > way. (This would be a great discussion topic for dev-platform, I doubt all those who are interested are reading this bug. The goal here was to fix what was broken by the recent tree-wide reformat.)
Comment 9•5 years ago
|
||
Tedc started this discussion: https://groups.google.com/forum/#!topic/mozilla.dev.platform/16PBaXgDlsA
Comment 10•5 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/e4712449ba43
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
status-firefox65:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 65
Updated•5 years ago
|
Target Milestone: Firefox 65 → mozilla65
Updated•2 years ago
|
Product: Firefox Build System → Developer Infrastructure
You need to log in
before you can comment on or make changes to this bug.
Description
•