Bug 72483 – Add .dir-locals.el file for Emacs

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Andy Wingo]

Emacs 23 and later support directory-local variables:

  http://www.gnu.org/s/libtool/manual/emacs/Directory-Variables.html

I occasionally see patches fail style checks because of indentation.  Adding a .dir-locals file to the repo would help avoid these, and as it's invisible to a normal `ls', it shouldn't be a burden on anyone.

Patch to be attached.

[Andy Wingo]

Created attachment 115353 [details]
Patch

[Andy Wingo]

Created attachment 115374 [details]
Patch

[Andy Wingo]

Updated with explicit blocks for c-mode and c++-mode, so that we really do override a user's default settings.

[Martin Robinson]

Comment on attachment 115353 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=115353&action=review

> .dir-locals.el:6
> +  (fill-column . 80)

The most common range I've heard is 120-180, but definitely not 80: https://lists.webkit.org/pipermail/webkit-dev/2009-September/009761.html

> .dir-locals.el:7
> +  (tab-width . 8)

Why not 4 here?

[Andy Wingo]

I must admit that I do not understand WebKit's predilection for long lines.  I assumed that people producing long lines were not using Emacs.  But I am fine putting in some other value.

Regarding tab widths, the docs say:

  Documentation:
  *Distance between tab stops (for display of tab characters), in columns.

That is the amount that TAB characters are interpreted as.  TAB characters are not usually present in WK, but where they are, as in Makefiles, they should probably be represented as 8 characters wide.  It is the default though, so it can probably be elided.

Do you have concrete recommendations?

[Martin Robinson]

(In reply to comment #5)
> I must admit that I do not understand WebKit's predilection for long lines.  I assumed that people producing long lines were not using Emacs.  But I am fine putting in some other value.

I would say either 180 or having no maximum.  At least in vim it can sometimes be annoying when the editor folds a pre-existing line when you modify it. Perosonally, I'm not a fan of long lines either, but the source contains a lot of code with very long lines.

> That is the amount that TAB characters are interpreted as.  TAB characters are not usually present in WK, but where they are, as in Makefiles, they should probably be represented as 8 characters wide.  It is the default though, so it can probably be elided.

You can probably just omit it. The only place we have tab characters is in our automake files.

I think it'd be best if an emacs user gives the final r+. :)

[Xan Lopez]

Comment on attachment 115374 [details]
Patch

I think it's best to just omit the maximum for fill-column, WebKit has lots of really crazy-long lines. Other than that, looks great!

Emacs is the default GNU editor.

[Andy Wingo]

Created attachment 115396 [details]
Patch

[Andy Wingo]

Comment on attachment 115396 [details]
Patch

Removed the fill-column and tab-width bits.  Xan, cq?

[Xan Lopez]

Comment on attachment 115396 [details]
Patch

Bam.

[WebKit Review Bot]

Comment on attachment 115396 [details]
Patch

Clearing flags on attachment: 115396

Committed r100902: <http://trac.webkit.org/changeset/100902>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Tommy Widenflycht]

I'm not particularly happy with this since it interferes my my existing .dir-local.el that have something similar but also modifications to the compile command :/

But if it does something useful I guess I can live with it.

[Martin Robinson]

Perhaps there can be a .dir.locals.locals.el or something similar to allow extending the WebKit defaults.

[Joshua Bell]

(In reply to comment #14)
> Perhaps there can be a .dir.locals.locals.el or something similar to allow extending the WebKit defaults.

http://code.google.com/p/chromium/wiki/Emacs 

... recommends having .dir-locals.el specify (c-file-style . "WebKit") for c-mode/c++-mode so that additional customization can be done in your .emacs - this would at least be a standard way to allow users to make additional style changes.

While I'm skeptical that editor-specific style options should be baked into the tree like this, I'd personally be satisfied (for now) if the c-file-style entries were added.

[Andy Wingo]

(In reply to comment #15)
> http://code.google.com/p/chromium/wiki/Emacs 
> 
> ... recommends having .dir-locals.el specify (c-file-style . "WebKit")

That's an interesting page, thanks for the link.

There are definitely things you can do to improve webkit editing if you are willing to edit your .emacs, but my goal with this was to make things better for people that don't have .emacs customizations.

If you are editing your .emacs, something like:

(defun webkit-c++-mode ()
  "C++ mode with adjusted defaults for use with WebKit."
  (interactive)
  (c++-mode)
  (c-set-style "WebKit")
  (setq indent-tabs-mode t)
  (setq c-basic-offset 4))

(setq auto-mode-alist (cons '("/WebKit/.*\\.[ch]*$" . webkit-c++-mode)
      auto-mode-alist))

[Andy Wingo]

Properly indented ;)

(setq auto-mode-alist (cons '("/WebKit/.*\\.[ch]*$" . webkit-c++-mode)
                      auto-mode-alist))