[Qemu-devel] [PATCH] Add .dir-locals.el file to configure emacs coding style

[Qemu-devel] [PATCH] Add .dir-locals.el file to configure emacs coding style

[Daniel P. Berrange]

The default emacs setup indents by 2 spaces and uses tabs
which is counter to the QEMU coding style rules. Adding a
.dir-locals.el file in the top level of the GIT repo will
inform emacs about the QEMU coding style, and so assist
contributors in avoiding common style mistakes before
they submit patches.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 .dir-locals.el | 8 ++++++++
 1 file changed, 8 insertions(+)
 create mode 100644 .dir-locals.el

diff --git a/.dir-locals.el b/.dir-locals.el
new file mode 100644
index 0000000..ddb2fae
--- /dev/null
+++ b/.dir-locals.el
@@ -0,0 +1,8 @@
+(
+ (c-mode . (
+            (c-file-style . "K&R")
+            (indent-tabs-mode . nil)
+            (c-indent-level . 4)
+            (c-basic-offset . 4)
+            ))
+)
-- 
2.4.1

Re: [Qemu-devel] [PATCH] Add .dir-locals.el file to configure emacs coding style

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 796 bytes --]

On 06/02/2015 09:26 AM, Daniel P. Berrange wrote:
> The default emacs setup indents by 2 spaces and uses tabs
> which is counter to the QEMU coding style rules. Adding a
> .dir-locals.el file in the top level of the GIT repo will
> inform emacs about the QEMU coding style, and so assist
> contributors in avoiding common style mistakes before
> they submit patches.
> 
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
>  .dir-locals.el | 8 ++++++++
>  1 file changed, 8 insertions(+)
>  create mode 100644 .dir-locals.el

Since we already have .exrc to do the same for vim users, I'm all in
favor of this.

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH] Add .dir-locals.el file to configure emacs coding style

[John Snow]

On 06/02/2015 12:02 PM, Eric Blake wrote:
> On 06/02/2015 09:26 AM, Daniel P. Berrange wrote:
>> The default emacs setup indents by 2 spaces and uses tabs
>> which is counter to the QEMU coding style rules. Adding a
>> .dir-locals.el file in the top level of the GIT repo will
>> inform emacs about the QEMU coding style, and so assist
>> contributors in avoiding common style mistakes before
>> they submit patches.
>>
>> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
>> ---
>>  .dir-locals.el | 8 ++++++++
>>  1 file changed, 8 insertions(+)
>>  create mode 100644 .dir-locals.el
> 
> Since we already have .exrc to do the same for vim users, I'm all in
> favor of this.
> 

+1

> Reviewed-by: Eric Blake <eblake@redhat.com>
> 

Reviewed-by: John Snow <jsnow@redhat.com>

Re: [Qemu-devel] [PATCH] Add .dir-locals.el file to configure emacs coding style

[Markus Armbruster]

"Daniel P. Berrange" <berrange@redhat.com> writes:

> The default emacs setup indents by 2 spaces and uses tabs
> which is counter to the QEMU coding style rules. Adding a
> .dir-locals.el file in the top level of the GIT repo will
> inform emacs about the QEMU coding style, and so assist
> contributors in avoiding common style mistakes before
> they submit patches.

Yes, please!

> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
>  .dir-locals.el | 8 ++++++++
>  1 file changed, 8 insertions(+)
>  create mode 100644 .dir-locals.el
>
> diff --git a/.dir-locals.el b/.dir-locals.el
> new file mode 100644
> index 0000000..ddb2fae
> --- /dev/null
> +++ b/.dir-locals.el
> @@ -0,0 +1,8 @@
> +(
> + (c-mode . (
> +            (c-file-style . "K&R")
> +            (indent-tabs-mode . nil)
> +            (c-indent-level . 4)
> +            (c-basic-offset . 4)
> +            ))
> +)

Unidiomatic placement of parenthesis.

The documented style name is "k&r", not "K&R".

This is its definition in my Emacs (23.4.1):

    ("k&r"
     (c-basic-offset . 5)
     (c-comment-only-line-offset . 0)
     (c-offsets-alist . ((statement-block-intro . +)
			 (knr-argdecl-intro . 0)
			 (substatement-open . 0)
			 (substatement-label . 0)
			 (label . 0)
			 (statement-cont . +))))

You overwrite c-basic-offset.  But then you can just as well use style
"stroustrup":

    ("stroustrup"
     (c-basic-offset . 4)
     (c-comment-only-line-offset . 0)
     (c-offsets-alist . ((statement-block-intro . +)
			 (substatement-open . 0)
			 (substatement-label . 0)
			 (label . 0)
			 (statement-cont . +))))

What does c-indent-level do?  Are you sure it's needed?

Does the following .dir-locals.el work for you equally well?

((c-mode . ((c-file-style . "stroustrup")
            (indent-tabs-mode . nil))))

Re: [Qemu-devel] [PATCH] Add .dir-locals.el file to configure emacs coding style

[Daniel P. Berrange]

On Wed, Jun 03, 2015 at 09:47:24AM +0200, Markus Armbruster wrote:
> "Daniel P. Berrange" <berrange@redhat.com> writes:
> 
> > The default emacs setup indents by 2 spaces and uses tabs
> > which is counter to the QEMU coding style rules. Adding a
> > .dir-locals.el file in the top level of the GIT repo will
> > inform emacs about the QEMU coding style, and so assist
> > contributors in avoiding common style mistakes before
> > they submit patches.
> 
> Yes, please!
> 
> > Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> > ---
> >  .dir-locals.el | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> >  create mode 100644 .dir-locals.el
> >
> > diff --git a/.dir-locals.el b/.dir-locals.el
> > new file mode 100644
> > index 0000000..ddb2fae
> > --- /dev/null
> > +++ b/.dir-locals.el
> > @@ -0,0 +1,8 @@
> > +(
> > + (c-mode . (
> > +            (c-file-style . "K&R")
> > +            (indent-tabs-mode . nil)
> > +            (c-indent-level . 4)
> > +            (c-basic-offset . 4)
> > +            ))
> > +)

[snip]

> Does the following .dir-locals.el work for you equally well?
> 
> ((c-mode . ((c-file-style . "stroustrup")
>             (indent-tabs-mode . nil))))

Yes, that is fine too.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [Qemu-trivial] [PATCH] Add .dir-locals.el file to configure emacs coding style

[Don Slutz]

On 06/04/15 09:30, Daniel P. Berrange wrote:
> On Wed, Jun 03, 2015 at 09:47:24AM +0200, Markus Armbruster wrote:
>> "Daniel P. Berrange" <berrange@redhat.com> writes:
>>
>>> The default emacs setup indents by 2 spaces and uses tabs
>>> which is counter to the QEMU coding style rules. Adding a
>>> .dir-locals.el file in the top level of the GIT repo will
>>> inform emacs about the QEMU coding style, and so assist
>>> contributors in avoiding common style mistakes before
>>> they submit patches.
>>
>> Yes, please!
>>
>>> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
>>> ---
>>>  .dir-locals.el | 8 ++++++++
>>>  1 file changed, 8 insertions(+)
>>>  create mode 100644 .dir-locals.el
>>>
>>> diff --git a/.dir-locals.el b/.dir-locals.el
>>> new file mode 100644
>>> index 0000000..ddb2fae
>>> --- /dev/null
>>> +++ b/.dir-locals.el
>>> @@ -0,0 +1,8 @@
>>> +(
>>> + (c-mode . (
>>> +            (c-file-style . "K&R")
>>> +            (indent-tabs-mode . nil)
>>> +            (c-indent-level . 4)
>>> +            (c-basic-offset . 4)
>>> +            ))
>>> +)
> 
> [snip]
> 
>> Does the following .dir-locals.el work for you equally well?
>>
>> ((c-mode . ((c-file-style . "stroustrup")
>>             (indent-tabs-mode . nil))))
> 
> Yes, that is fine too.
> 

I got this from someone on the list, see

https://wiki.linaro.org/PeterMaydell/QemuEmacsStyle

Which then allows me to do:

((c-mode . ((c-file-style . "qemu"))))

in .dir-locals.el

But this is not a good general change :(

But I would like at least the the stroustrup version.

   -Don Slutz

> Regards,
> Daniel
> 

Re: [Qemu-devel] [Qemu-trivial] [PATCH] Add .dir-locals.el file to configure emacs coding style

[Don Slutz]

On 06/04/15 09:30, Daniel P. Berrange wrote:
> On Wed, Jun 03, 2015 at 09:47:24AM +0200, Markus Armbruster wrote:
>> "Daniel P. Berrange" <berrange@redhat.com> writes:
>>
>>> The default emacs setup indents by 2 spaces and uses tabs
>>> which is counter to the QEMU coding style rules. Adding a
>>> .dir-locals.el file in the top level of the GIT repo will
>>> inform emacs about the QEMU coding style, and so assist
>>> contributors in avoiding common style mistakes before
>>> they submit patches.
>>
>> Yes, please!
>>
>>> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
>>> ---
>>>  .dir-locals.el | 8 ++++++++
>>>  1 file changed, 8 insertions(+)
>>>  create mode 100644 .dir-locals.el
>>>
>>> diff --git a/.dir-locals.el b/.dir-locals.el
>>> new file mode 100644
>>> index 0000000..ddb2fae
>>> --- /dev/null
>>> +++ b/.dir-locals.el
>>> @@ -0,0 +1,8 @@
>>> +(
>>> + (c-mode . (
>>> +            (c-file-style . "K&R")
>>> +            (indent-tabs-mode . nil)
>>> +            (c-indent-level . 4)
>>> +            (c-basic-offset . 4)
>>> +            ))
>>> +)
> 
> [snip]
> 
>> Does the following .dir-locals.el work for you equally well?
>>
>> ((c-mode . ((c-file-style . "stroustrup")
>>             (indent-tabs-mode . nil))))
> 
> Yes, that is fine too.
> 

I got this from someone on the list, see

https://wiki.linaro.org/PeterMaydell/QemuEmacsStyle

Which then allows me to do:

((c-mode . ((c-file-style . "qemu"))))

in .dir-locals.el

But this is not a good general change :(

But I would like at least the the stroustrup version.

   -Don Slutz

> Regards,
> Daniel
> 

Re: [Qemu-devel] [PATCH] Add .dir-locals.el file to configure emacs coding style

[Michael Tokarev]

So, what is the consensus here?

Everyone who talked wants the emacs mode, but everyone
offers their own mode.

I'd pick the stroustrup variant suggested by Marcus
since it is shortest, but while being shortest, it
is looks a bit "magical".  On the other hand, variant
from Peter Maydell (https://wiki.linaro.org/PeterMaydell/QemuEmacsStyle)
explicitly defines everything.

Thanks,

/mjt

02.06.2015 18:26, Daniel P. Berrange wrote:
> The default emacs setup indents by 2 spaces and uses tabs
> which is counter to the QEMU coding style rules. Adding a
> .dir-locals.el file in the top level of the GIT repo will
> inform emacs about the QEMU coding style, and so assist
> contributors in avoiding common style mistakes before
> they submit patches.
> 
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
>  .dir-locals.el | 8 ++++++++
>  1 file changed, 8 insertions(+)
>  create mode 100644 .dir-locals.el
> 
> diff --git a/.dir-locals.el b/.dir-locals.el
> new file mode 100644
> index 0000000..ddb2fae
> --- /dev/null
> +++ b/.dir-locals.el
> @@ -0,0 +1,8 @@
> +(
> + (c-mode . (
> +            (c-file-style . "K&R")
> +            (indent-tabs-mode . nil)
> +            (c-indent-level . 4)
> +            (c-basic-offset . 4)
> +            ))
> +)
> 

Re: [Qemu-devel] [PATCH] Add .dir-locals.el file to configure emacs coding style

[Markus Armbruster]

Michael Tokarev <mjt@tls.msk.ru> writes:

> So, what is the consensus here?
>
> Everyone who talked wants the emacs mode, but everyone
> offers their own mode.
>
> I'd pick the stroustrup variant suggested by Marcus
> since it is shortest, but while being shortest, it
> is looks a bit "magical".

I don't think it's magical at all.  It uses .dir-locals exactly as
intended.  In fact, it's almost straight from the Emacs manual:

       The '.dir-locals.el' file should hold a specially-constructed list,
    which maps major mode names (symbols) to alists (*note
    (elisp)Association Lists::).  Each alist entry consists of a variable
    name and the directory-local value to assign to that variable, when the
    specified major mode is enabled.  Instead of a mode name, you can
    specify 'nil', which means that the alist applies to any mode; or you
    can specify a subdirectory name (a string), in which case the alist
    applies to all files in that subdirectory.

       Here's an example of a '.dir-locals.el' file:

         ((nil . ((indent-tabs-mode . t)
                  (fill-column . 80)))
          (c-mode . ((c-file-style . "BSD")
                     (subdirs . nil)))
          ("src/imported"
           . ((nil . ((change-log-default-name
                       . "ChangeLog.local"))))))

    This sets 'indent-tabs-mode' and 'fill-column' for any file in the
    directory tree, and the indentation style for any C source file.  The
    special 'subdirs' element is not a variable, but a special keyword which
    indicates that the C mode settings are only to be applied in the current
    directory, not in any subdirectories.  Finally, it specifies a different
    'ChangeLog' file name for any file in the 'src/imported' subdirectory.

The .dir-locals.el snippet I suggested is a straighforward cherry-pick
from the above:

    ((c-mode . ((c-file-style . "stroustrup")
                (indent-tabs-mode . nil))))

Here's one that takes better care of tabs:

    ((nil . ((indent-tabs-mode . nil)))
     (makefile-mode ((indent-tabs-mode . t)))
     (c-mode . ((c-file-style . "stroustrup"))))

>                            On the other hand, variant
> from Peter Maydell (https://wiki.linaro.org/PeterMaydell/QemuEmacsStyle)
> explicitly defines everything.

I'm afraid putting this into the source tree isn't as easy, because it
involves defining a new style, which you're not supposed do in
.dir-locals.el (it's for directory local variables, not for defining
global constants and calling functions).

So users would have to put the style definition in their .emacs, and its
use in .dir-locals.el.  If we then commit the latter to the repository,
we screw everybody who hasn't added the former to his .emacs.  No go.

We could try something like

    ((nil . ((indent-tabs-mode . nil)))
     (makefile-mode ((indent-tabs-mode . t)))
     (c-mode . ((c-file-style . (if (assoc "qemu" c-style-alist)
                                    "qemu" "stroustrup")))))

but that triggers the "contains values that may not be safe" prompt, so
it's another no go.

Let's see what Peter's style adds to "stroustrup", to gauge how much
trouble getting it would be worth:

(defconst qemu-c-style
  '(
    ;; recommend to do indent-tabs-mode separately to cover other major modes
    (indent-tabs-mode . nil)
    ;; same as stroustrup
    (c-basic-offset . 4)
    ;; this is the default, and anyone changing it is nuts
    (tab-width . 8)
    ;; default
    (c-comment-only-line-offset . 0)
    ;; duplicate entry, the one below wins, this one has no effect
    (c-hanging-braces-alist . ((substatement-open before after)))
    (c-offsets-alist . ((statement-block-intro . +)
                        (substatement-open . 0)
                        (label . 0)
                        (statement-cont . +)
			;; up to here same as stroustrup
			;; except we don't have (substatement-label . 0)
			;; and thus default to (substatement-label . 2)
			;;
			;; stroustrup has + instead of 0
			;; C++ "namespace" blocks
			;; do we care to differ from stroustrup?
                        (innamespace . 0)
			;; stroustrup has + instead of 0
			;; Brace that opens an in-class inline method
			;; do we care to differ from stroustrup?
                        (inline-open . 0)
                        ))
    ;; This isn't for indentation, it's for automatically inserting
    ;; newlines when you type braces in auto-newline minor mode.
    (c-hanging-braces-alist .
                            (
			     ;; same as stroustrup
			     (brace-list-open)
			     ;; only qemu
			     ;; First line in an enum or static array list
			     ;; suppress auto-newlines there
                             (brace-list-intro)
			     ;; only qemu
			     ;; Subsequent lines in an enum or static array
			     ;; suppress auto-newlines there
                             (brace-list-entry)
			     ;; only qemu
			     ;; Close brace of an enum or static array list.
			     ;; suppress auto-newlines there
                             (brace-list-close)
			     ;; same as stroustrup
                             (brace-entry-open)
			     ;; same as stroustrup
                             (block-close . c-snug-do-while)
			     ;; only qemu
			     ;; Brace that opens a class definition
			     ;; auto-newline after the brace
			     ;; stroustrup instead has
			     ;; (inexpr-class-open after)
			     ;; applies to anonymous inner classes in Java
                             (class-open . (after))
			     ;; same as stroustrup
                             (substatement-open . (after))
			     ;; only qemu
			     ;; Brace that opens a class definition
			     ;; suppress auto-newlines there
			     ;; stroustrup instead has
			     ;; (inexpr-class-close before)
			     ;; applies to anonymous inner classes in Java
                             (class-close)
			     ;; stroustrup additionally has
			     ;; A continuation of a C (or like) statement.
			     ;; (statement-cont)
			     ;; Brace that opens an "extern" block
			     ;; (extern-lang-open after)
			     ;; Likewise, C++ "namespace" block
			     ;; (namespace-open after)
			     ;; Likewise, CORBA IDL "module" block
			     ;; (module-open after)
			     ;; Likewise, CORBA CIDL "composition" block
			     ;; (composition-open after)
			     ;; Subsequent argument list lines when at
			     ;; least one argument follows on the same
			     ;; line as the arglist opening paren
			     ;; (arglist-cont-nonempty)
                             ))
    )
  "QEMU C Programming Style")

I can't find anything major.

Want me to post a formal patch adding my revised .dir-locals.el?

Re: [Qemu-devel] [PATCH] Add .dir-locals.el file to configure emacs coding style

[Michael Tokarev]

18.06.2015 11:36, Markus Armbruster wrote:
> Michael Tokarev <mjt@tls.msk.ru> writes:
> 
>> So, what is the consensus here?
>>
>> Everyone who talked wants the emacs mode, but everyone
>> offers their own mode.
>>
>> I'd pick the stroustrup variant suggested by Marcus
>> since it is shortest, but while being shortest, it
>> is looks a bit "magical".
> 
> I don't think it's magical at all.  It uses .dir-locals exactly as
> intended.  In fact, it's almost straight from the Emacs manual:

No, I mean the one-line "stroustrup" definition is a bit "magical"
as it "magically" have all parameters behind the scenes matching
our coding style, as opposed to listing every aspect explicitly.

[]
> Want me to post a formal patch adding my revised .dir-locals.el?

Daniel already posted his 2-line V2.  It might be good
idea to add indent-tabs-mod nil to other modes too.

At any rate I'm *far* from emacs expert (even if it was my
only editor and development environment for about 10 years,
I forgot almost everything already).

Thanks,

/mjt