| Message ID | 1433258768-28946-1-git-send-email-berrange@redhat.com |
|---|---|
| State | New |
| Headers | show |
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>
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>
"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))))
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
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 >
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 >
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) > + )) > +) >
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?
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
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) + )) +)
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