619113 - Invalid vim modeline in nsDocShell.cpp

Status: RESOLVED FIXED

(Core :: DOM: Navigation, defect)

Description

[Justin Lebar (not reading bugmail)]

When opening nsDocShell.cpp in Vim, I get:

Error detected while processing modelines:
line    2:
E518: Unknown option: */

The line in question is:

>  /* vim: ft=cpp tw=78 sw=4 et ts=8 sts=4 cin */

But should be

>  /* vim: set ft=cpp tw=78 sw=4 et ts=8 sts=4 cin: */

or alternatively shouldn't have the trailing "*/".

Broken in http://hg.mozilla.org/mozilla-central/rev/4222674401ba

Comment 1

[Justin Lebar (not reading bugmail)]

Attachment: Patch

Comment 2

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Comment on attachment 497535 [details] [diff] [review]
Patch

r=me, if you make it

/* vim: set ts=2 et sw=2 tw=80: */

per <https://developer.mozilla.org/En/Mozilla_Coding_Style_Guide#Mode_Line> unless this one is better; in that case, please update MDC.

Comment 3

[Justin Lebar (not reading bugmail)]

docshell uses a combination of two-space and four-space tabstops.  But looking through the file, most of them are four spaces, so I think the modeline should reflect that with

  sw=4 sts=4

There aren't any tabs in the file, so the ts setting doesn't matter; I'm happy to set it to 4.  But then we'd probably want to modify the emacs modeline.

I think we want |et| (expand tabs) and |cin| (indent as C code).  I can add those to the style guide, although we should probably get someone else's eyes on the change first.

Comment 4

[Justin Lebar (not reading bugmail)]

For what it's worth, we have approximately zero consistency in our modelines.

$ hg locate | egrep 'cpp$|h$' | xargs grep -h 'vim:' | uniq -c | sort -rn | head -n20
     37  * vim: set ts=4 sw=4 et tw=99:
     34  * vim: sw=2 ts=2 et lcs=trail\:.,tab\:>~ :
     34 /* vim: set ts=2 et sw=2 tw=80: */
     33 /* vim:set ts=2 sw=2 sts=2 et cindent: */
     28 /* vim:expandtab:shiftwidth=4:tabstop=4:
     27 /* vim:expandtab:shiftwidth=2:tabstop=2:
     26  * vim: set ts=8 sw=4 et tw=99:
     21 /* vim:set ts=2 sw=2 sts=2 et cindent: */
     16  * vim: sw=2 ts=8 et :
     14  * vim: sw=4 ts=4 et :
     14  * vim: set ts=8 sw=4 et tw=79:
     14 /* vim:set ts=2 sw=2 sts=2 et cindent: */
     12  * vim: sw=2 ts=2 et lcs=trail\:.,tab\:>~ :
     12 /* vim:expandtab:shiftwidth=4:tabstop=4:
     11  * vim: sw=2 ts=8 et :
     11 /* vim: se cin sw=2 ts=2 et : */
     11 /* vim:expandtab:shiftwidth=2:tabstop=2:
     10  * vim: set ts=4 sw=4 et tw=99 ft=cpp:
      9  * vim: sw=4 ts=4 et :
      9 /* vim:set ts=2 sw=2 sts=2 et cindent: */

Comment 5

[Justin Lebar (not reading bugmail)]

$ hg locate | egrep 'cpp$|h$' | xargs grep -h 'vim:' | wc -l
1202

Comment 6

[Justin Lebar (not reading bugmail)]

Proposed compromise for docshell so I can check something in:

/* vim: set ts=4 sw=4 et tw=80: */

Comment 7

[Justin Lebar (not reading bugmail)]

Attachment: Patch v2

Comment 8

[Justin Lebar (not reading bugmail)]

Oh no!  I made the newb mistake of forgetting to sort before uniq.

These results suggest that |et cin| might not be so bad to leave in the modeline.

$ hg locate | egrep 'cpp$|h$' | xargs grep -h 'vim:' | sort | uniq -c | sort -rn | head -n20
    146 /* vim:set ts=2 sw=2 sts=2 et cindent: */
     97 /* vim:expandtab:shiftwidth=4:tabstop=4:
     69 /* vim:expandtab:shiftwidth=2:tabstop=2:
     65  * vim: sw=2 ts=2 et lcs=trail\:.,tab\:>~ :
     51  * vim: sw=4 ts=4 et :
     48  * vim: sw=2 ts=8 et :
     46  * vim: set ts=4 sw=4 et tw=99:
     43  * vim: set ts=8 sw=4 et tw=99:
     43 /* vim: set ts=2 et sw=2 tw=80: */
     33 /* vim: set ts=2 sw=2 et tw=78: */
     26 /* vim: set ts=2 sw=2 et tw=80: */
     25 /* vim: set sw=2 ts=8 et tw=80 : */
     22 // vim:cindent:ts=2:et:sw=2:
     20  * vim: set ts=8 sw=4 et tw=79:
     20  * vim: set ts=8 sw=4 et tw=78:
     18  * vim: set ts=4 sw=4 et tw=99 ft=cpp:
     17 /* vim: se cin sw=2 ts=2 et : */
     15 /* vim:set ts=4 sw=4 sts=4 et cin: */
     14  * vim: sw=2 ts=2 et :
     14 /* vim:set ts=4 sw=4 et cindent: */

Comment 9

[Justin Lebar (not reading bugmail)]

http://hg.mozilla.org/mozilla-central/rev/1ea92ceef5f0