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)

enhancement
Not set
normal

Tracking

(firefox65 fixed)

RESOLVED FIXED
mozilla65
Tracking Status
firefox65 --- fixed

People

(Reporter: bbouvier, Assigned: bbouvier)

References

Details

Attachments

(1 file, 1 obsolete file)

      No description provided.
Attached patch fix.patch (obsolete) — Splinter Review
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: nobody → bbouvier
Status: NEW → ASSIGNED
Attachment #9028951 - Flags: review?(sledru)
Blocks: 1511393
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)
I wonder if we should just remove those mode lines. They are often inconsistent with each other.
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.
Attached patch fix.patchSplinter Review
That also includes changing the tab width to 2 spaces everywhere.
Attachment #9028951 - Attachment is obsolete: true
Attachment #9028977 - Flags: review?(sledru)
Summary: Update vim modelines to wrap lines after 79 chars → Update vim modelines to wrap lines after 80 chars
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
(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.)
https://hg.mozilla.org/mozilla-central/rev/e4712449ba43
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 65
Target Milestone: Firefox 65 → mozilla65
Product: Firefox Build System → Developer Infrastructure
You need to log in before you can comment on or make changes to this bug.