|
From: Andreas A. <ar...@li...> - 2018-11-21 16:47:26
|
Hi, After receiving feedback from Bart Van Assche and Mark Wielaard (thanks for that), here's an update to the proposed patch for adding Emacs configuration files to the Valgrind source tree. Does this now sufficiently cover all coding styles used in Valgrind? -- Andreas -- >8 -- Subject: [PATCH] Add Emacs configuration files This adds a configuration file ".dir-locals.el" for Emacs to the topmost directory of the Valgrind source tree, and another such file to the directory drd/tests. These files contain per-directory local Emacs variables. The following settings are performed: * The base C style is set to "Linux", indentation is set to 3 columns per level, the use of tabs for indentation is disabled, and the fill column is set to 80. * The directory coregrind/m_demangle is handled specially. The files in it are imported from libiberty and thus marked read-only, except for "demangle.c" and "vg_libciface.h", which are specific to Valgrind. * The source files in drd/tests use 2 instead of 3 columns per indentation level. --- .dir-locals.el | 24 ++++++++++++++++++++++++ coregrind/m_demangle/demangle.c | 1 + coregrind/m_demangle/vg_libciface.h | 1 + drd/tests/.dir-locals.el | 16 ++++++++++++++++ 4 files changed, 42 insertions(+) create mode 100644 .dir-locals.el create mode 100644 drd/tests/.dir-locals.el diff --git a/.dir-locals.el b/.dir-locals.el new file mode 100644 index 000000000..a10d51304 --- /dev/null +++ b/.dir-locals.el @@ -0,0 +1,24 @@ +;; Emacs settings for Valgrind. + +( + ;; Format used to turn a bug number into a URL (when displaying it). + (nil . ((bug-reference-url-format . "http://bugs.kde.org/show_bug.cgi?id=%s"))) + + ;; Standard settings for editing C source files. + (c-mode . ( + ;; Apply the Linux style as a base. + (c-file-style . "linux") + ;; Use spaces instead of tabs for indentation. + (indent-tabs-mode . nil) + ;; Indent 3 columns per level. + (c-basic-offset . 3) + ;; Lines should be limited to 80 columns. + (fill-column . 80) + )) + + ("coregrind/m_demangle" + ;; Most files in this directory should not be edited; they are imported + ;; from libiberty through the auxprogs/update-demangler script. + . ((nil . ((buffer-read-only . t)))) + ) +) diff --git a/coregrind/m_demangle/demangle.c b/coregrind/m_demangle/demangle.c index 00fa20389..dd933da8b 100644 --- a/coregrind/m_demangle/demangle.c +++ b/coregrind/m_demangle/demangle.c @@ -1,3 +1,4 @@ +/* -*- mode: C; buffer-read-only: nil -*- */ /*--------------------------------------------------------------------*/ /*--- Demangling of C++ mangled names. demangle.c ---*/ diff --git a/coregrind/m_demangle/vg_libciface.h b/coregrind/m_demangle/vg_libciface.h index 762e63a02..1aefd5db3 100644 --- a/coregrind/m_demangle/vg_libciface.h +++ b/coregrind/m_demangle/vg_libciface.h @@ -1,3 +1,4 @@ +/* -*- mode: C; buffer-read-only: nil -*- */ /*--------------------------------------------------------------------*/ /*--- Demangling of C++ mangled names. vg_libciface.h ---*/ diff --git a/drd/tests/.dir-locals.el b/drd/tests/.dir-locals.el new file mode 100644 index 000000000..0d94ded88 --- /dev/null +++ b/drd/tests/.dir-locals.el @@ -0,0 +1,16 @@ +;; Emacs settings for drd tests. + +( + ;; Standard settings for editing C source files. + (c-mode . ( + ;; Apply the Linux style as a base. + (c-file-style . "linux") + ;; Use spaces instead of tabs for indentation. + (indent-tabs-mode . nil) + ;; Indent 2 columns per level. Note that this differs from + ;; the usual Valgrind style. + (c-basic-offset . 2) + ;; Lines should be limited to 80 columns. + (fill-column . 80) + )) +) -- 2.17.0 |
|
From: Bart V. A. <bva...@ac...> - 2018-12-01 20:22:39
|
On 11/21/18 8:47 AM, Andreas Arnez wrote: > diff --git a/coregrind/m_demangle/demangle.c b/coregrind/m_demangle/demangle.c > index 00fa20389..dd933da8b 100644 > --- a/coregrind/m_demangle/demangle.c > +++ b/coregrind/m_demangle/demangle.c > @@ -1,3 +1,4 @@ > +/* -*- mode: C; buffer-read-only: nil -*- */ > > /*--------------------------------------------------------------------*/ > /*--- Demangling of C++ mangled names. demangle.c ---*/ > diff --git a/coregrind/m_demangle/vg_libciface.h b/coregrind/m_demangle/vg_libciface.h > index 762e63a02..1aefd5db3 100644 > --- a/coregrind/m_demangle/vg_libciface.h > +++ b/coregrind/m_demangle/vg_libciface.h > @@ -1,3 +1,4 @@ > +/* -*- mode: C; buffer-read-only: nil -*- */ Can this be left out? I'd like to avoid mode lines in source files. > diff --git a/drd/tests/.dir-locals.el b/drd/tests/.dir-locals.el > new file mode 100644 > index 000000000..0d94ded88 > --- /dev/null > +++ b/drd/tests/.dir-locals.el > @@ -0,0 +1,16 @@ > +;; Emacs settings for drd tests. > + > +( > + ;; Standard settings for editing C source files. > + (c-mode . ( > + ;; Apply the Linux style as a base. > + (c-file-style . "linux") > + ;; Use spaces instead of tabs for indentation. > + (indent-tabs-mode . nil) > + ;; Indent 2 columns per level. Note that this differs from > + ;; the usual Valgrind style. > + (c-basic-offset . 2) > + ;; Lines should be limited to 80 columns. > + (fill-column . 80) > + )) > +) Is it possible to combine the top-level .dir-locals.el file and this file into a single file? If not, don't worry. How about Helgrind? Are files in helgrind/tests indented by 2 or by 3 columns? Is a similar file needed in helgrind/tests? Sorry for being slow in replying. Thanks, Bart. |
|
From: Andreas A. <ar...@li...> - 2018-12-03 19:13:27
|
On Sat, Dec 01 2018, Bart Van Assche wrote: > On 11/21/18 8:47 AM, Andreas Arnez wrote: >> diff --git a/coregrind/m_demangle/demangle.c b/coregrind/m_demangle/demangle.c >> index 00fa20389..dd933da8b 100644 >> --- a/coregrind/m_demangle/demangle.c >> +++ b/coregrind/m_demangle/demangle.c >> @@ -1,3 +1,4 @@ >> +/* -*- mode: C; buffer-read-only: nil -*- */ >> /*--------------------------------------------------------------------*/ >> /*--- Demangling of C++ mangled names. demangle.c ---*/ >> diff --git a/coregrind/m_demangle/vg_libciface.h b/coregrind/m_demangle/vg_libciface.h >> index 762e63a02..1aefd5db3 100644 >> --- a/coregrind/m_demangle/vg_libciface.h >> +++ b/coregrind/m_demangle/vg_libciface.h >> @@ -1,3 +1,4 @@ >> +/* -*- mode: C; buffer-read-only: nil -*- */ > > Can this be left out? I'd like to avoid mode lines in source files. Not sure. Do you know another way to exempt these files from being marked read-only? > >> diff --git a/drd/tests/.dir-locals.el b/drd/tests/.dir-locals.el >> new file mode 100644 >> index 000000000..0d94ded88 >> --- /dev/null >> +++ b/drd/tests/.dir-locals.el >> @@ -0,0 +1,16 @@ >> +;; Emacs settings for drd tests. >> + >> +( >> + ;; Standard settings for editing C source files. >> + (c-mode . ( >> + ;; Apply the Linux style as a base. >> + (c-file-style . "linux") >> + ;; Use spaces instead of tabs for indentation. >> + (indent-tabs-mode . nil) >> + ;; Indent 2 columns per level. Note that this differs from >> + ;; the usual Valgrind style. >> + (c-basic-offset . 2) >> + ;; Lines should be limited to 80 columns. >> + (fill-column . 80) >> + )) >> +) > > Is it possible to combine the top-level .dir-locals.el file and this file > into a single file? If not, don't worry. If you know how, I'd be interested... > > How about Helgrind? Are files in helgrind/tests indented by 2 or by 3 > columns? Is a similar file needed in helgrind/tests? The C source files in that directory use various different indentation widths, sometimes even within the same file. I haven't measured which of these occurs most often, but it's certainly not obvious. So maybe we could just as well postulate that these files should converge to Valgrind's usual style. What do you think? > > Sorry for being slow in replying. > > Thanks, > > Bart. |
|
From: Bart V. A. <bva...@ac...> - 2018-12-03 19:35:27
|
On Mon, 2018-12-03 at 20:11 +0100, Andreas Arnez wrote:
> On Sat, Dec 01 2018, Bart Van Assche wrote:
> > On 11/21/18 8:47 AM, Andreas Arnez wrote:
> > > diff --git a/coregrind/m_demangle/demangle.c b/coregrind/m_demangle/demangle.c
> > > index 00fa20389..dd933da8b 100644
> > > --- a/coregrind/m_demangle/demangle.c
> > > +++ b/coregrind/m_demangle/demangle.c
> > > @@ -1,3 +1,4 @@
> > > +/* -*- mode: C; buffer-read-only: nil -*- */
> > > /*--------------------------------------------------------------------*/
> > > /*--- Demangling of C++ mangled names. demangle.c ---*/
> > > diff --git a/coregrind/m_demangle/vg_libciface.h b/coregrind/m_demangle/vg_libciface.h
> > > index 762e63a02..1aefd5db3 100644
> > > --- a/coregrind/m_demangle/vg_libciface.h
> > > +++ b/coregrind/m_demangle/vg_libciface.h
> > > @@ -1,3 +1,4 @@
> > > +/* -*- mode: C; buffer-read-only: nil -*- */
> >
> > Can this be left out? I'd like to avoid mode lines in source files.
>
> Not sure. Do you know another way to exempt these files from being
> marked read-only?
Are these modelines necessary since your patch adds the following to
.dir-locals.el?
+ ("coregrind/m_demangle"
+ ;; Most files in this directory should not be edited; they are imported
+ ;; from libiberty through the auxprogs/update-demangler script.
+ . ((nil . ((buffer-read-only . t))))
+ )
> > How about Helgrind? Are files in helgrind/tests indented by 2 or by 3
> > columns? Is a similar file needed in helgrind/tests?
>
> The C source files in that directory use various different indentation
> widths, sometimes even within the same file. I haven't measured which
> of these occurs most often, but it's certainly not obvious. So maybe we
> could just as well postulate that these files should converge to
> Valgrind's usual style. What do you think?
That sounds fine to me. But since Julian is maintaining Helgrind, Julian
has the last word about Helgrind test indentation.
Bart.
|
|
From: Andreas A. <ar...@li...> - 2018-12-04 09:31:37
|
On Mon, Dec 03 2018, Bart Van Assche wrote:
> On Mon, 2018-12-03 at 20:11 +0100, Andreas Arnez wrote:
[...]
>> Not sure. Do you know another way to exempt these files from being
>> marked read-only?
>
> Are these modelines necessary since your patch adds the following to
> .dir-locals.el?
>
> + ("coregrind/m_demangle"
> + ;; Most files in this directory should not be edited; they are imported
> + ;; from libiberty through the auxprogs/update-demangler script.
> + . ((nil . ((buffer-read-only . t))))
> + )
Right.
>
>> > How about Helgrind? Are files in helgrind/tests indented by 2 or by 3
>> > columns? Is a similar file needed in helgrind/tests?
>>
>> The C source files in that directory use various different indentation
>> widths, sometimes even within the same file. I haven't measured which
>> of these occurs most often, but it's certainly not obvious. So maybe we
>> could just as well postulate that these files should converge to
>> Valgrind's usual style. What do you think?
>
> That sounds fine to me. But since Julian is maintaining Helgrind, Julian
> has the last word about Helgrind test indentation.
Sure.
--
Andreas
|
|
From: Patrick J. L. <lop...@gm...> - 2018-12-03 20:37:24
|
On Mon, Dec 3, 2018 at 11:14 AM Andreas Arnez <ar...@li...> wrote: > > Not sure. Do you know another way to exempt these files from being > marked read-only? > ... > > Is it possible to combine the top-level .dir-locals.el file and this file > > into a single file? If not, don't worry. If you know how, I'd be interested... > According to the documentation: https://www.gnu.org/software/emacs/manual/html_node/emacs/Directory-Variables.html ...a top-level .dir-locals.el can specify settings for particular subdirectories. - Pat |
|
From: Andreas A. <ar...@li...> - 2018-12-04 09:24:21
|
On Mon, Dec 03 2018, Patrick J. LoPresti wrote: > On Mon, Dec 3, 2018 at 11:14 AM Andreas Arnez <ar...@li...> wrote: > > Not sure. Do you know another way to exempt these files from being > marked read-only? > > ... > > > Is it possible to combine the top-level .dir-locals.el file and this file > > into a single file? If not, don't worry. > > If you know how, I'd be interested... > > According to the documentation: > > https://www.gnu.org/software/emacs/manual/html_node/emacs/Directory-Variables.html > > ...a top-level .dir-locals.el can specify settings for particular subdirectories. Yes, I know that. This is exclusive of specifying settings for specific modes, though. Or maybe I overlooked something. Can you provide an example that does what we want? -- Andreas |
|
From: Patrick J. L. <lop...@gm...> - 2018-12-05 01:21:40
|
On Tue, Dec 4, 2018 at 1:24 AM Andreas Arnez <ar...@li...> wrote: > > > > > According to the documentation: > > > > https://www.gnu.org/software/emacs/manual/html_node/emacs/Directory-Variables.html > > > > ...a top-level .dir-locals.el can specify settings for particular subdirectories. > > Yes, I know that. This is exclusive of specifying settings for specific > modes, though. Or maybe I overlooked something. Can you provide an > example that does what we want? The example in the documentation: ((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")))))) ...provides settings for src/imported that apply for any mode, but that is just this example... If you replace the nil (under "src/imported") with a mode name, it should apply only for that directory and that mode, I think. - Pat |
|
From: Andreas A. <ar...@li...> - 2018-12-05 12:31:22
|
On Tue, Dec 04 2018, Patrick J. LoPresti wrote:
> The example in the documentation:
>
> ((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"))))))
>
> ...provides settings for src/imported that apply for any mode, but
> that is just this example... If you replace the nil (under
> "src/imported") with a mode name, it should apply only for that
> directory and that mode, I think.
No, if I do that and keep the global mode-specific setting, then the
latter seems to be applied unconditionally. Do you know a way to avoid
that?
--
Andreas
|
|
From: Bart V. A. <bva...@ac...> - 2018-12-05 15:02:46
|
On 12/5/18 4:30 AM, Andreas Arnez wrote:
> On Tue, Dec 04 2018, Patrick J. LoPresti wrote:
>
>> The example in the documentation:
>>
>> ((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"))))))
>>
>> ...provides settings for src/imported that apply for any mode, but
>> that is just this example... If you replace the nil (under
>> "src/imported") with a mode name, it should apply only for that
>> directory and that mode, I think.
>
> No, if I do that and keep the global mode-specific setting, then the
> latter seems to be applied unconditionally. Do you know a way to avoid
> that?
If nobody comes up with a solution, please repost v2 of your patch but
without the emacs modelines.
Thanks,
Bart.
|
|
From: Andreas A. <ar...@li...> - 2018-12-05 16:07:21
|
On Wed, Dec 05 2018, Bart Van Assche wrote: > If nobody comes up with a solution, please repost v2 of your patch but > without the emacs modelines. So I guess we drop the idea of marking the imported files read-only, right? Or did you have a better option? Anyway, I'm OK with that. It basically means that the directory coregrind/m_demangle is not handled specially at all. New version below. -- Andreas -- >8 -- Subject: [PATCH v3] Add Emacs configuration files This adds a configuration file ".dir-locals.el" for Emacs to the topmost directory of the Valgrind source tree, and another such file to the directory drd/tests. These files contain per-directory local Emacs variables. The following settings are performed: * The base C style is set to "Linux", indentation is set to 3 columns per level, the use of tabs for indentation is disabled, and the fill column is set to 80. * The source files in drd/tests use 2 instead of 3 columns per indentation level. --- .dir-locals.el | 18 ++++++++++++++++++ drd/tests/.dir-locals.el | 16 ++++++++++++++++ 2 files changed, 34 insertions(+) create mode 100644 .dir-locals.el create mode 100644 drd/tests/.dir-locals.el diff --git a/.dir-locals.el b/.dir-locals.el new file mode 100644 index 000000000..5957ed944 --- /dev/null +++ b/.dir-locals.el @@ -0,0 +1,18 @@ +;; Emacs settings for Valgrind. + +( + ;; Format used to turn a bug number into a URL (when displaying it). + (nil . ((bug-reference-url-format . "http://bugs.kde.org/show_bug.cgi?id=%s"))) + + ;; Standard settings for editing C source files. + (c-mode . ( + ;; Apply the Linux style as a base. + (c-file-style . "linux") + ;; Use spaces instead of tabs for indentation. + (indent-tabs-mode . nil) + ;; Indent 3 columns per level. + (c-basic-offset . 3) + ;; Lines should be limited to 80 columns. + (fill-column . 80) + )) +) diff --git a/drd/tests/.dir-locals.el b/drd/tests/.dir-locals.el new file mode 100644 index 000000000..0d94ded88 --- /dev/null +++ b/drd/tests/.dir-locals.el @@ -0,0 +1,16 @@ +;; Emacs settings for drd tests. + +( + ;; Standard settings for editing C source files. + (c-mode . ( + ;; Apply the Linux style as a base. + (c-file-style . "linux") + ;; Use spaces instead of tabs for indentation. + (indent-tabs-mode . nil) + ;; Indent 2 columns per level. Note that this differs from + ;; the usual Valgrind style. + (c-basic-offset . 2) + ;; Lines should be limited to 80 columns. + (fill-column . 80) + )) +) -- 2.17.0 |
|
From: Bart V. A. <bva...@ac...> - 2018-12-06 02:17:28
|
On 12/5/18 8:07 AM, Andreas Arnez wrote: > Subject: [PATCH v3] Add Emacs configuration files v3 has been applied. Thanks for the patch! Bart. |