[PATCH -mm] documentation: update CodingStyle tips for Emacs users

[PATCH -mm] documentation: update CodingStyle tips for Emacs users

[Johannes Weiner]

Describe a setup that integrates better with Emacs' cc-mode and also
fixes up the alignment of continuation lines to really only use tabs.

Signed-off-by: Johannes Weiner <hannes@saeurebad.de>
---

Sorry, forgot linux-kernel on first send.

diff --git a/Documentation/CodingStyle b/Documentation/CodingStyle
index 6caa146..4148e9f 100644
--- a/Documentation/CodingStyle
+++ b/Documentation/CodingStyle
@@ -474,25 +474,29 @@ make a good program).
 So, you can either get rid of GNU emacs, or change it to use saner
 values.  To do the latter, you can stick the following in your .emacs file:
 
-(defun linux-c-mode ()
-  "C mode with adjusted defaults for use with the Linux kernel."
-  (interactive)
-  (c-mode)
-  (c-set-style "K&R")
-  (setq tab-width 8)
-  (setq indent-tabs-mode t)
-  (setq c-basic-offset 8))
-
-This will define the M-x linux-c-mode command.  When hacking on a
-module, if you put the string -*- linux-c -*- somewhere on the first
-two lines, this mode will be automatically invoked. Also, you may want
-to add
-
-(setq auto-mode-alist (cons '("/usr/src/linux.*/.*\\.[ch]$" . linux-c-mode)
-			auto-mode-alist))
-
-to your .emacs file if you want to have linux-c-mode switched on
-automagically when you edit source files under /usr/src/linux.
+(defun c-lineup-arglist-tabs-only (ignored)
+  "Line up argument lists by tabs, not spaces"
+  (let* ((anchor (c-langelem-pos c-syntactic-element))
+	 (column (c-langelem-2nd-pos c-syntactic-element))
+	 (offset (- (1+ column) anchor))
+	 (steps (floor offset c-basic-offset)))
+    (* (max steps 1)
+       c-basic-offset)))
+
+(add-hook 'c-mode-hook
+          (lambda ()
+            (let ((filename (buffer-file-name)))
+              ;; Enable kernel mode for the appropriate files
+              (when (and file
+                         (string-match "/usr/src/linux" filename))
+                (setq indent-tabs-mode t)
+                (c-set-style "linux")
+                (c-set-offset 'arglist-cont-nonempty
+                              '(c-lineup-gcc-asm-reg
+                                c-lineup-arglist-tabs-only))))))
+
+This will make emacs go better with the kernel coding style for C
+files in /usr/src/linux.
 
 But even if you fail in getting emacs to do sane formatting, not
 everything is lost: use "indent".

Re: [PATCH -mm] documentation: update CodingStyle tips for Emacs users

[Jonathan Corbet]

On Fri, 04 Jul 2008 19:52:16 +0200
Johannes Weiner <hannes@saeurebad.de> wrote:

> +(add-hook 'c-mode-hook
> +          (lambda ()
> +            (let ((filename (buffer-file-name)))
> +              ;; Enable kernel mode for the appropriate files
> +              (when (and file
> +                         (string-match "/usr/src/linux" filename))

Shouldn't that be "(and filename ..." rather than "file"?

A test for /usr/src/linux seems misplaced, though; I don't think a
whole lot of people keep their kernel trees there anymore.  There must
be a better heuristic one can use to figure out whether a kernel source
file is being edited.

jon

Re: [PATCH -mm] documentation: update CodingStyle tips for Emacs users

[Johannes Weiner]

Hi,

Jonathan Corbet <corbet@lwn.net> writes:

> On Fri, 04 Jul 2008 19:52:16 +0200
> Johannes Weiner <hannes@saeurebad.de> wrote:
>
>> +(add-hook 'c-mode-hook
>> +          (lambda ()
>> +            (let ((filename (buffer-file-name)))
>> +              ;; Enable kernel mode for the appropriate files
>> +              (when (and file
>> +                         (string-match "/usr/src/linux" filename))
>
> Shouldn't that be "(and filename ..." rather than "file"?

Whoops, you are right.

> A test for /usr/src/linux seems misplaced, though; I don't think a
> whole lot of people keep their kernel trees there anymore.  There must
> be a better heuristic one can use to figure out whether a kernel source
> file is being edited.

Is this really needed?  I think most people have their kernel trees in
one directory and I used /usr/src/linux explicitely because it is so
well known and whoever reads this snippet knows what to substitute it
for.

But perhaps it should be "~/src/linux-trees/" to emphasize that it it
matches everything descendant from this directory.

A heuristics seems overkill as this snippet is not distributed code that
needs to be portable but it is just a tip for ones .emacs.  I don't want
heuristics in my .emacs if I know exactly where my kernel trees are.

But then, we should remove the check for linux-files completely and make
people use this coding style for every code they write ;)

	Hannes

Re: [PATCH -mm] documentation: update CodingStyle tips for Emacs users

[Jonathan Corbet]

On Sat, 05 Jul 2008 11:36:44 +0000
Johannes Weiner <hannes@saeurebad.de> wrote:

> A heuristics seems overkill as this snippet is not distributed code
> that needs to be portable but it is just a tip for ones .emacs. 

You're right, it's fine the way it is.

jon

[PATCH -mm] documentation: update CodingStyle tips for Emacs users v2

[Johannes Weiner]

Hi,

Jonathan Corbet <corbet@lwn.net> writes:

> On Sat, 05 Jul 2008 11:36:44 +0000
> Johannes Weiner <hannes@saeurebad.de> wrote:
>
>> A heuristics seems overkill as this snippet is not distributed code
>> that needs to be portable but it is just a tip for ones .emacs. 
>
> You're right, it's fine the way it is.

Okay, I fixed the file->filename mistake.  How about the following?

	Hannes

---
From: Johannes Weiner <hannes@saeurebad.de>
Subject: documentation: update CodingStyle tips for Emacs users

Describe a setup that integrates better with Emacs' cc-mode and also
fixes up the alignment of continuation lines to really only use tabs.

Signed-off-by: Johannes Weiner <hannes@saeurebad.de>
CC: Jonathan Corbet <corbet@lwn.net>
---

diff --git a/Documentation/CodingStyle b/Documentation/CodingStyle
index 6caa146..1875e50 100644
--- a/Documentation/CodingStyle
+++ b/Documentation/CodingStyle
@@ -474,25 +474,29 @@ make a good program).
 So, you can either get rid of GNU emacs, or change it to use saner
 values.  To do the latter, you can stick the following in your .emacs file:
 
-(defun linux-c-mode ()
-  "C mode with adjusted defaults for use with the Linux kernel."
-  (interactive)
-  (c-mode)
-  (c-set-style "K&R")
-  (setq tab-width 8)
-  (setq indent-tabs-mode t)
-  (setq c-basic-offset 8))
-
-This will define the M-x linux-c-mode command.  When hacking on a
-module, if you put the string -*- linux-c -*- somewhere on the first
-two lines, this mode will be automatically invoked. Also, you may want
-to add
-
-(setq auto-mode-alist (cons '("/usr/src/linux.*/.*\\.[ch]$" . linux-c-mode)
-			auto-mode-alist))
-
-to your .emacs file if you want to have linux-c-mode switched on
-automagically when you edit source files under /usr/src/linux.
+(defun c-lineup-arglist-tabs-only (ignored)
+  "Line up argument lists by tabs, not spaces"
+  (let* ((anchor (c-langelem-pos c-syntactic-element))
+	 (column (c-langelem-2nd-pos c-syntactic-element))
+	 (offset (- (1+ column) anchor))
+	 (steps (floor offset c-basic-offset)))
+    (* (max steps 1)
+       c-basic-offset)))
+
+(add-hook 'c-mode-hook
+          (lambda ()
+            (let ((filename (buffer-file-name)))
+              ;; Enable kernel mode for the appropriate files
+              (when (and filename
+                         (string-match "~/src/linux-trees" filename))
+                (setq indent-tabs-mode t)
+                (c-set-style "linux")
+                (c-set-offset 'arglist-cont-nonempty
+                              '(c-lineup-gcc-asm-reg
+                                c-lineup-arglist-tabs-only))))))
+
+This will make emacs go better with the kernel coding style for C
+files below ~/src/linux-trees.
 
 But even if you fail in getting emacs to do sane formatting, not
 everything is lost: use "indent".

Re: [PATCH -mm] documentation: update CodingStyle tips for Emacs users

[Patrick Ohly]

On Fri, 2008-07-04 at 19:52 +0200, Johannes Weiner wrote:
> Describe a setup that integrates better with Emacs' cc-mode and also
> fixes up the alignment of continuation lines to really only use tabs.
[...]
> +(defun c-lineup-arglist-tabs-only (ignored)
> +  "Line up argument lists by tabs, not spaces"
> +  (let* ((anchor (c-langelem-pos c-syntactic-element))
> +	 (column (c-langelem-2nd-pos c-syntactic-element))
> +	 (offset (- (1+ column) anchor))
> +	 (steps (floor offset c-basic-offset)))
> +    (* (max steps 1)
> +       c-basic-offset)))
>+
>+(add-hook 'c-mode-hook
> +          (lambda ()
> +            (let ((filename (buffer-file-name)))
> +              ;; Enable kernel mode for the appropriate files
> +              (when (and file
> +                         (string-match "/usr/src/linux" filename))
> +                (setq indent-tabs-mode t)
> +                (c-set-style "linux")
> +                (c-set-offset 'arglist-cont-nonempty
> +                              '(c-lineup-gcc-asm-reg
> +                                c-lineup-arglist-tabs-only))))))

When starting writing kernel source code last year (the hardware time
stamping patch) I used this macro to format it. I very much liked the
possibility to only switch on Linux style when editing Linux code
(because I work on a variety of code bases with different formatting).

But the macro above did not produce the result expected by some patch
reviewers (John Stultz, Dave Miller). I had to redo the formatting
manually.

The sticky point is continuation after splitting a long parameter list -
in other words, exactly what the "arglist-cont-nonempty" setting above
changes. The macro formats it so that the continuation is aligned at the
tab that is closest to the initial argument in the previous line:

int a_very_long_function_name(int arg1, char *arg2,
			double arg3);

The expected style was tab plus spaces to align with the first argument:

int a_very_long_function_name(int arg1, char *arg2,
			      double arg3);

Is this a bug in the macro? I'm using emacs 22.1.1, in case that
matters. Or are there different opinions in the Linux developer
community about the right formatting?

-- 
Best Regards, Patrick Ohly

The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.

Re: [PATCH -mm] documentation: update CodingStyle tips for Emacs users

[Johannes Weiner]

On Wed, Feb 18, 2009 at 10:19:30AM +0100, Patrick Ohly wrote:
> On Fri, 2008-07-04 at 19:52 +0200, Johannes Weiner wrote:
> > Describe a setup that integrates better with Emacs' cc-mode and also
> > fixes up the alignment of continuation lines to really only use tabs.
> [...]
> > +(defun c-lineup-arglist-tabs-only (ignored)
> > +  "Line up argument lists by tabs, not spaces"
> > +  (let* ((anchor (c-langelem-pos c-syntactic-element))
> > +	 (column (c-langelem-2nd-pos c-syntactic-element))
> > +	 (offset (- (1+ column) anchor))
> > +	 (steps (floor offset c-basic-offset)))
> > +    (* (max steps 1)
> > +       c-basic-offset)))
> >+
> >+(add-hook 'c-mode-hook
> > +          (lambda ()
> > +            (let ((filename (buffer-file-name)))
> > +              ;; Enable kernel mode for the appropriate files
> > +              (when (and file
> > +                         (string-match "/usr/src/linux" filename))
> > +                (setq indent-tabs-mode t)
> > +                (c-set-style "linux")
> > +                (c-set-offset 'arglist-cont-nonempty
> > +                              '(c-lineup-gcc-asm-reg
> > +                                c-lineup-arglist-tabs-only))))))
> 
> When starting writing kernel source code last year (the hardware time
> stamping patch) I used this macro to format it. I very much liked the
> possibility to only switch on Linux style when editing Linux code
> (because I work on a variety of code bases with different formatting).
> 
> But the macro above did not produce the result expected by some patch
> reviewers (John Stultz, Dave Miller). I had to redo the formatting
> manually.
> 
> The sticky point is continuation after splitting a long parameter list -
> in other words, exactly what the "arglist-cont-nonempty" setting above
> changes. The macro formats it so that the continuation is aligned at the
> tab that is closest to the initial argument in the previous line:
> 
> int a_very_long_function_name(int arg1, char *arg2,
> 			double arg3);
> 
> The expected style was tab plus spaces to align with the first argument:
> 
> int a_very_long_function_name(int arg1, char *arg2,
> 			        double arg3);
> 
> Is this a bug in the macro? I'm using emacs 22.1.1, in case that
> matters. Or are there different opinions in the Linux developer
> community about the right formatting?

Unfortunately, this seems to depend on the maintainer.  Both styles
are used all over the kernel but Documentation/CodingStyle and
scripts/checkpatch.pl prefer what the above emacs configuration does.

I doubt that people do tab tab tab space space space space space by
hand, so I strongly suspect that the tab-only zealots don't use emacs
which does this adjustment automatically.

You should probably add linux-davem to the above or something.  Or
name the coding style after the text-editor the target maintainer
uses :-)

	Hannes

Re: [PATCH -mm] documentation: update CodingStyle tips for Emacs users

[Patrick Ohly]

On Wed, 2009-02-18 at 09:59 +0000, Johannes Weiner wrote:
[formatting of line continuation in argument list]
> Unfortunately, this seems to depend on the maintainer.  Both styles
> are used all over the kernel but Documentation/CodingStyle and
> scripts/checkpatch.pl prefer what the above emacs configuration does.

checkpatch.pl didn't seem to mind either way. I haven't seen this
particular aspect mentioned in the CodingStyle either.

> I doubt that people do tab tab tab space space space space space by
> hand, so I strongly suspect that the tab-only zealots don't use emacs
> which does this adjustment automatically.
> 
> You should probably add linux-davem to the above or something.  Or
> name the coding style after the text-editor the target maintainer
> uses :-)

One could also get really fancy and extend the path matching to pick the
right style depending on the subdirectory ;-)

Thanks for the clarification. In the future I'll pay closer attention to
existing code and turn the macro on or off accordingly.

-- 
Best Regards, Patrick Ohly

The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.

Re: [PATCH -mm] documentation: update CodingStyle tips for Emacs users

[Johannes Weiner]

On Wed, Feb 18, 2009 at 11:23:59AM +0100, Patrick Ohly wrote:
> On Wed, 2009-02-18 at 09:59 +0000, Johannes Weiner wrote:
> [formatting of line continuation in argument list]
> > Unfortunately, this seems to depend on the maintainer.  Both styles
> > are used all over the kernel but Documentation/CodingStyle and
> > scripts/checkpatch.pl prefer what the above emacs configuration does.
> 
> checkpatch.pl didn't seem to mind either way. I haven't seen this
> particular aspect mentioned in the CodingStyle either.

This is from CodingStyle:

	Outside of comments, documentation and except in Kconfig,
	spaces are never used for indentation, and the above example
	is deliberately broken.

> > I doubt that people do tab tab tab space space space space space by
> > hand, so I strongly suspect that the tab-only zealots don't use emacs
> > which does this adjustment automatically.
> > 
> > You should probably add linux-davem to the above or something.  Or
> > name the coding style after the text-editor the target maintainer
> > uses :-)
> 
> One could also get really fancy and extend the path matching to pick the
> right style depending on the subdirectory ;-)
> 
> Thanks for the clarification. In the future I'll pay closer attention to
> existing code and turn the macro on or off accordingly.

That's probably the best way to judge such details, agreed.  Integrate
well with the surrounding code.

	Hannes