bug#55039: [PATCH] Use VC-relative file names in ChangeLog entries

bug#55039: [PATCH] Use VC-relative file names in ChangeLog entries

[Philip Kaludercic]

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

Tags: patch


By default C-c C-w (log-edit-generate-changelog-from-diff) just uses the
file name when generating a changelog message.  As it has been requested
to use the project relative path name when writing changelog messages,
it seems to make sense to generate these automatically as well.

Alternatively, this could also be configured using a directory local
variable that would be set for emacs.git.


In GNU Emacs 29.0.50 (build 6, x86_64-pc-linux-gnu, GTK+ Version 3.24.30, cairo version 1.16.0)
 of 2022-04-19 built on viero
Repository revision: d168decab4a03cd48b8b96257c36ffa575c720e6
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12011000
System Description: Guix System

Configured using:
 'configure
 PKG_CONFIG_PATH=/home/philip/.guix-profile/lib/pkgconfig:/home/philip/.guix-profile/share/pkgconfig'


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Use-VC-relative-file-names-in-ChangeLog-entries.patch --]
[-- Type: text/patch, Size: 1285 bytes --]

From f3f87cab56d1bc1754d9e83a9dd53d0cb48c02b0 Mon Sep 17 00:00:00 2001
From: Philip Kaludercic <philipk@posteo.net>
Date: Tue, 30 Nov 2021 21:05:42 +0100
Subject: [PATCH] Use VC-relative file names in ChangeLog entries

* lisp/vc/diff-mode.el (diff-add-log-current-defuns): Check
vc-root-dir to prepend the project path to the file being modified.

See https://mail.gnu.org/archive/html/emacs-devel/2021-11/msg02195.html
---
 lisp/vc/diff-mode.el | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/lisp/vc/diff-mode.el b/lisp/vc/diff-mode.el
index 057ffcd06e..40749754d8 100644
--- a/lisp/vc/diff-mode.el
+++ b/lisp/vc/diff-mode.el
@@ -2343,7 +2343,10 @@ diff-add-log-current-defuns
           ;; hunks (e.g., "diff --git ..." etc).
           (re-search-forward diff-hunk-header-re nil t)
         (setq hunk-end (save-excursion (diff-end-of-hunk)))
-        (pcase-let* ((filename (substring-no-properties (diff-find-file-name)))
+        (pcase-let* ((filename (substring-no-properties
+                                (file-relative-name
+                                   (diff-find-file-name)
+                                   (vc-root-dir))))
                      (=lines 0)
                      (+lines 0)
                      (-lines 0)
-- 
2.34.0


[-- Attachment #3: Type: text/plain, Size: 24 bytes --]


-- 
	Philip Kaludercic

bug#55039: [PATCH] Use VC-relative file names in ChangeLog entries

[Eli Zaretskii]

> From: Philip Kaludercic <philipk@posteo.net>
> Date: Wed, 20 Apr 2022 14:19:39 +0000
> 
> By default C-c C-w (log-edit-generate-changelog-from-diff) just uses the
> file name when generating a changelog message.  As it has been requested
> to use the project relative path name when writing changelog messages,
> it seems to make sense to generate these automatically as well.
> 
> Alternatively, this could also be configured using a directory local
> variable that would be set for emacs.git.

First, I think this must be customizable, because different projects
have different conventions.

Second, I think the code should DTRT if there's already a ChangeLog
file in the current directory or in one of its parent directories,
because those usually show you the project's conventions.

bug#55039: [PATCH] Use VC-relative file names in ChangeLog entries

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> First, I think this must be customizable, because different projects
> have different conventions.

This was several months ago -- Philip, did you do any further work in
this direction?

> Second, I think the code should DTRT if there's already a ChangeLog
> file in the current directory or in one of its parent directories,
> because those usually show you the project's conventions.

This would be a separate issue, though?  (And I agree that it would be
nice to have, but it doesn't sound trivial to implement.)

bug#55039: [PATCH] Use VC-relative file names in ChangeLog entries

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: Philip Kaludercic <philipk@posteo.net>,  55039@debbugs.gnu.org
> Date: Tue, 06 Sep 2022 13:13:01 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Second, I think the code should DTRT if there's already a ChangeLog
> > file in the current directory or in one of its parent directories,
> > because those usually show you the project's conventions.
> 
> This would be a separate issue, though?  (And I agree that it would be
> nice to have, but it doesn't sound trivial to implement.)

The proposed feature isn't optional, it unconditionally changes the
behavior.  AFAIU, it will change the way file names are mentioned in
log messages regardless of anything.  Does it sound right to you?

IOW, providing a new feature in response to a request is fine, but why
immediately make it the default, and why force it on everyone without
any fire escape?

So if we are not going to teach this feature to look at exiting
ChangeLog files, at the very l;east this feature should be optional,
perhaps off by default.

And btw, this only changes the behavior of one command, but AFAIK
there are other commands that produce ChangeLog-style entries -- what
about them?

bug#55039: [PATCH] Use VC-relative file names in ChangeLog entries

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

>> This would be a separate issue, though?  (And I agree that it would be
>> nice to have, but it doesn't sound trivial to implement.)
>
> The proposed feature isn't optional, it unconditionally changes the
> behavior.  AFAIU, it will change the way file names are mentioned in
> log messages regardless of anything.  Does it sound right to you?

No, I agreed with you that the proposed patch needs work.  I just
pointed out that your second paragraph was a separate issue, because the
code doesn't heed existing ChangeLog files anyway.

bug#55039: [PATCH] Use VC-relative file names in ChangeLog entries

[Philip Kaludercic]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Eli Zaretskii <eliz@gnu.org> writes:
>
>> First, I think this must be customizable, because different projects
>> have different conventions.
>
> This was several months ago -- Philip, did you do any further work in
> this direction?

No I hadn't, but I can take tackle it again as this shouldn't be that
difficult.

>> Second, I think the code should DTRT if there's already a ChangeLog
>> file in the current directory or in one of its parent directories,
>> because those usually show you the project's conventions.
>
> This would be a separate issue, though?  (And I agree that it would be
> nice to have, but it doesn't sound trivial to implement.)

bug#55039: [PATCH] Use VC-relative file names in ChangeLog entries

[Philip Kaludercic]

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

Philip Kaludercic <philipk@posteo.net> writes:

> Lars Ingebrigtsen <larsi@gnus.org> writes:
>
>> Eli Zaretskii <eliz@gnu.org> writes:
>>
>>> First, I think this must be customizable, because different projects
>>> have different conventions.
>>
>> This was several months ago -- Philip, did you do any further work in
>> this direction?
>
> No I hadn't, but I can take tackle it again as this shouldn't be that
> difficult.

Something like this should be enough, at least functionality wise,
right?


[-- Attachment #2: Type: text/plain, Size: 1355 bytes --]

diff --git a/lisp/vc/diff-mode.el b/lisp/vc/diff-mode.el
index 322d4abd31..1f0eec0d66 100644
--- a/lisp/vc/diff-mode.el
+++ b/lisp/vc/diff-mode.el
@@ -2336,6 +2336,12 @@ diff-undo
   (let ((inhibit-read-only t))
     (undo arg)))
 
+(defcustom diff-relative-names-in-changelog nil
+  "Use relative file names when generating ChangeLog messages."
+  :type 'boolean
+  :safe #'booleanp
+  :version "29.1")
+
 (defun diff-add-log-current-defuns ()
   "Return an alist of defun names for the current diff.
 The elements of the alist are of the form (FILE . (DEFUN...)),
@@ -2374,9 +2380,11 @@ diff-add-log-current-defuns
           (re-search-forward diff-hunk-header-re nil t)
         (setq hunk-end (save-excursion (diff-end-of-hunk)))
         (pcase-let* ((filename (substring-no-properties
-                                (file-relative-name
-                                   (diff-find-file-name)
-                                   (vc-root-dir))))
+                                (if diff-relative-names-in-changelog
+                                    (file-relative-name
+                                     (diff-find-file-name)
+                                     (vc-root-dir))
+                                  (diff-find-file-name))))
                      (=lines 0)
                      (+lines 0)
                      (-lines 0)

[-- Attachment #3: Type: text/plain, Size: 261 bytes --]


Assuming nobody objects to the name of the option, should this be
documented in NEWS and/or in the Manual?  Also, as the entire point of
the patch was that emacs.git is using relative path names, should a
.dir-locals.el variable be set to ensure this is done?

bug#55039: [PATCH] Use VC-relative file names in ChangeLog entries

[Lars Ingebrigtsen]

Philip Kaludercic <philipk@posteo.net> writes:

> Something like this should be enough, at least functionality wise,
> right?

Looks good to me.

> Assuming nobody objects to the name of the option, should this be
> documented in NEWS and/or in the Manual?  Also, as the entire point of
> the patch was that emacs.git is using relative path names, 

Should be in NEWS and mentioned in the `diff-add-log-current-defuns' doc
string, I think.  I don't know whether it's necessary to document it in
the manual.

> should a .dir-locals.el variable be set to ensure this is done?

I think so.

bug#55039: [PATCH] Use VC-relative file names in ChangeLog entries

[Robert Pluim]

>>>>> On Tue, 06 Sep 2022 14:34:58 +0000, Philip Kaludercic <philipk@posteo.net> said:
    Philip>  (defun diff-add-log-current-defuns ()
    Philip>    "Return an alist of defun names for the current diff.
    Philip>  The elements of the alist are of the form (FILE . (DEFUN...)),
    Philip> @@ -2374,9 +2380,11 @@ diff-add-log-current-defuns
    Philip>            (re-search-forward diff-hunk-header-re nil t)
    Philip>          (setq hunk-end (save-excursion (diff-end-of-hunk)))
    Philip>          (pcase-let* ((filename (substring-no-properties
    Philip> -                                (file-relative-name
    Philip> -                                   (diff-find-file-name)
    Philip> -                                   (vc-root-dir))))
    Philip> +                                (if diff-relative-names-in-changelog
    Philip> +                                    (file-relative-name
    Philip> +                                     (diff-find-file-name)
    Philip> +                                     (vc-root-dir))
    Philip> +                                  (diff-find-file-name))))
    Philip>                       (=lines 0)
    Philip>                       (+lines 0)
    Philip>                       (-lines 0)

That doesnʼt look like the code thatʼs currently in master. Is this on
top of your previous patch?

    Philip> Assuming nobody objects to the name of the option, should this be
    Philip> documented in NEWS and/or in the Manual?  Also, as the entire point of
    Philip> the patch was that emacs.git is using relative path names, should a
    Philip> .dir-locals.el variable be set to ensure this is done?

NEWS and .dir-locals.el definitely. I guess you could add to the "Log
Buffer" node in the emacs manual if youʼre feeling inspired 😀

Robert
-- 

bug#55039: [PATCH] Use VC-relative file names in ChangeLog entries

[Philip Kaludercic]

Robert Pluim <rpluim@gmail.com> writes:

>>>>>> On Tue, 06 Sep 2022 14:34:58 +0000, Philip Kaludercic <philipk@posteo.net> said:
>     Philip>  (defun diff-add-log-current-defuns ()
>     Philip>    "Return an alist of defun names for the current diff.
>     Philip>  The elements of the alist are of the form (FILE . (DEFUN...)),
>     Philip> @@ -2374,9 +2380,11 @@ diff-add-log-current-defuns
>     Philip>            (re-search-forward diff-hunk-header-re nil t)
>     Philip>          (setq hunk-end (save-excursion (diff-end-of-hunk)))
>     Philip>          (pcase-let* ((filename (substring-no-properties
>     Philip> -                                (file-relative-name
>     Philip> -                                   (diff-find-file-name)
>     Philip> -                                   (vc-root-dir))))
>     Philip> +                                (if diff-relative-names-in-changelog
>     Philip> +                                    (file-relative-name
>     Philip> +                                     (diff-find-file-name)
>     Philip> +                                     (vc-root-dir))
>     Philip> +                                  (diff-find-file-name))))
>     Philip>                       (=lines 0)
>     Philip>                       (+lines 0)
>     Philip>                       (-lines 0)
>
> That doesnʼt look like the code thatʼs currently in master. Is this on
> top of your previous patch?

No, these changes would be on top of the previous patch, but I'm of
course going to merge them into a single commit.

>     Philip> Assuming nobody objects to the name of the option, should this be
>     Philip> documented in NEWS and/or in the Manual?  Also, as the entire point of
>     Philip> the patch was that emacs.git is using relative path names, should a
>     Philip> .dir-locals.el variable be set to ensure this is done?
>
> NEWS and .dir-locals.el definitely. I guess you could add to the "Log
> Buffer" node in the emacs manual if youʼre feeling inspired 😀

I'll consider it.

> Robert

bug#55039: [PATCH] Use VC-relative file names in ChangeLog entries

[Eli Zaretskii]

> From: Philip Kaludercic <philipk@posteo.net>
> Cc: Eli Zaretskii <eliz@gnu.org>,  55039@debbugs.gnu.org
> Date: Tue, 06 Sep 2022 14:34:58 +0000
> 
> +(defcustom diff-relative-names-in-changelog nil
> +  "Use relative file names when generating ChangeLog messages."

The doc string should reference the commands for which this option is
relevant.

> Assuming nobody objects to the name of the option, should this be
> documented in NEWS and/or in the Manual?

I think so, yes, since "C-x 4 A" is in the manual, and this option
will affect it, right?

> Also, as the entire point of
> the patch was that emacs.git is using relative path names, should a
> .dir-locals.el variable be set to ensure this is done?

I guess so, yes.  But what would happen to people who use Emacs
versions before 29?

bug#55039: [PATCH] Use VC-relative file names in ChangeLog entries

[Philip Kaludercic]

Eli Zaretskii <eliz@gnu.org> writes:

>> Assuming nobody objects to the name of the option, should this be
>> documented in NEWS and/or in the Manual?
>
> I think so, yes, since "C-x 4 A" is in the manual, and this option
> will affect it, right?

From what I see, it won't be affected.  Do you think it should be?

>> Also, as the entire point of
>> the patch was that emacs.git is using relative path names, should a
>> .dir-locals.el variable be set to ensure this is done?
>
> I guess so, yes.  But what would happen to people who use Emacs
> versions before 29?

It should have no effect.

bug#55039: [PATCH] Use VC-relative file names in ChangeLog entries

[Eli Zaretskii]

> From: Philip Kaludercic <philipk@posteo.net>
> Cc: larsi@gnus.org,  55039@debbugs.gnu.org
> Date: Tue, 06 Sep 2022 20:11:57 +0000
> 
> >> Also, as the entire point of
> >> the patch was that emacs.git is using relative path names, should a
> >> .dir-locals.el variable be set to ensure this is done?
> >
> > I guess so, yes.  But what would happen to people who use Emacs
> > versions before 29?
> 
> It should have no effect.

Unfortunately, this is not true, at least not the way this was
implemented in our .dir-locals.el.  Now, whenever I visit _any_ file
in the repository, I get this warning pop up:

  The local variables list in d:/gnu/git/emacs/trunk/
  contains values that may not be safe (*).

  Do you want to apply it?  You can type
  y  -- to apply the local variables list.
  n  -- to ignore the local variables list.
  !  -- to apply the local variables list, and permanently mark these
	values (*) as safe (in the future, they will be set automatically.)
  i  -- to ignore the local variables list, and permanently mark these
	values (*) as ignored

      tab-width : 8
      sentence-end-double-space : t
      fill-column : 70
      emacs-lisp-docstring-fill-column : 65
      bug-reference-url-format : "https://debbugs.gnu.org/%s"
    * diff-add-log-use-relative-names : t
      indent-tabs-mode : nil
      electric-quote-comment : nil
      electric-quote-string : nil
      mode : bug-reference-prog

and I _must_ type a response.  If I answer !, Emacs will write to my
init file, which I'm not sure I like; if I type y, I will have to do
that for every file I visit.

This is the nuisance caused by immediately adding newly introduced
variables into file-local or directory-local settings to our
repository.  We should keep this in mind when introducing such stuff.

bug#55039: [PATCH] Use VC-relative file names in ChangeLog entries

[Philip Kaludercic]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Philip Kaludercic <philipk@posteo.net>
>> Cc: larsi@gnus.org,  55039@debbugs.gnu.org
>> Date: Tue, 06 Sep 2022 20:11:57 +0000
>> 
>> >> Also, as the entire point of
>> >> the patch was that emacs.git is using relative path names, should a
>> >> .dir-locals.el variable be set to ensure this is done?
>> >
>> > I guess so, yes.  But what would happen to people who use Emacs
>> > versions before 29?
>> 
>> It should have no effect.
>
> Unfortunately, this is not true, at least not the way this was
> implemented in our .dir-locals.el.  Now, whenever I visit _any_ file
> in the repository, I get this warning pop up:
>
>   The local variables list in d:/gnu/git/emacs/trunk/
>   contains values that may not be safe (*).
>
>   Do you want to apply it?  You can type
>   y  -- to apply the local variables list.
>   n  -- to ignore the local variables list.
>   !  -- to apply the local variables list, and permanently mark these
> 	values (*) as safe (in the future, they will be set automatically.)
>   i  -- to ignore the local variables list, and permanently mark these
> 	values (*) as ignored
>
>       tab-width : 8
>       sentence-end-double-space : t
>       fill-column : 70
>       emacs-lisp-docstring-fill-column : 65
>       bug-reference-url-format : "https://debbugs.gnu.org/%s"
>     * diff-add-log-use-relative-names : t
>       indent-tabs-mode : nil
>       electric-quote-comment : nil
>       electric-quote-string : nil
>       mode : bug-reference-prog
>
> and I _must_ type a response.  If I answer !, Emacs will write to my
> init file, which I'm not sure I like; if I type y, I will have to do
> that for every file I visit.

I see, should I revert the change?

> This is the nuisance caused by immediately adding newly introduced
> variables into file-local or directory-local settings to our
> repository.  We should keep this in mind when introducing such stuff.

Generally speaking, the ability to set a variable only if the symbol is
bound would be useful for features like these.

bug#55039: [PATCH] Use VC-relative file names in ChangeLog entries

[Eli Zaretskii]

> From: Philip Kaludercic <philipk@posteo.net>
> Cc: larsi@gnus.org,  55039@debbugs.gnu.org
> Date: Thu, 08 Sep 2022 08:45:47 +0000
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> From: Philip Kaludercic <philipk@posteo.net>
> >> Cc: larsi@gnus.org,  55039@debbugs.gnu.org
> >> Date: Tue, 06 Sep 2022 20:11:57 +0000
> >> 
> >> >> Also, as the entire point of
> >> >> the patch was that emacs.git is using relative path names, should a
> >> >> .dir-locals.el variable be set to ensure this is done?
> >> >
> >> > I guess so, yes.  But what would happen to people who use Emacs
> >> > versions before 29?
> >> 
> >> It should have no effect.
> >
> > Unfortunately, this is not true, at least not the way this was
> > implemented in our .dir-locals.el.  Now, whenever I visit _any_ file
> > in the repository, I get this warning pop up:
> >
> >   The local variables list in d:/gnu/git/emacs/trunk/
> >   contains values that may not be safe (*).
> >
> >   Do you want to apply it?  You can type
> >   y  -- to apply the local variables list.
> >   n  -- to ignore the local variables list.
> >   !  -- to apply the local variables list, and permanently mark these
> > 	values (*) as safe (in the future, they will be set automatically.)
> >   i  -- to ignore the local variables list, and permanently mark these
> > 	values (*) as ignored
> >
> >       tab-width : 8
> >       sentence-end-double-space : t
> >       fill-column : 70
> >       emacs-lisp-docstring-fill-column : 65
> >       bug-reference-url-format : "https://debbugs.gnu.org/%s"
> >     * diff-add-log-use-relative-names : t
> >       indent-tabs-mode : nil
> >       electric-quote-comment : nil
> >       electric-quote-string : nil
> >       mode : bug-reference-prog
> >
> > and I _must_ type a response.  If I answer !, Emacs will write to my
> > init file, which I'm not sure I like; if I type y, I will have to do
> > that for every file I visit.
> 
> I see, should I revert the change?

Is it possible to make the setting dependent on the Emacs version?

I don't think there's a need to revert, even if we cannot make this
better.  Assuming people want this setting very much, that is.  Just
something to remember for the future: introducing these is not
cost-free.

> > This is the nuisance caused by immediately adding newly introduced
> > variables into file-local or directory-local settings to our
> > repository.  We should keep this in mind when introducing such stuff.
> 
> Generally speaking, the ability to set a variable only if the symbol is
> bound would be useful for features like these.

I don't think that's possible in practice, since many variables are
loaded only when needed.  But if I'm wrong, then yes, it would be good
to have such a capability.

bug#55039: [PATCH] Use VC-relative file names in ChangeLog entries

[Lars Ingebrigtsen]

Philip Kaludercic <philipk@posteo.net> writes:

> Generally speaking, the ability to set a variable only if the symbol is
> bound would be useful for features like these.

Yes, that would be very nice.  But I think introducing something like
that would be difficult...

Introduce a new syntax like

;; Local Variables:
;; foo-bar: t boundp
;; End:

is difficult to make backwards-compatible, but perhaps we can come up
with something that older Emacsen will ignore but newer ones can parse?

We could also introduce a new section for these variables:

;; Boundp Local Variables:
;; foo-bar: t
;; End:

But...  I'm not much fan of that.

bug#55039: [PATCH] Use VC-relative file names in ChangeLog entries

[Visuwesh]

[வியாழன் செப்டம்பர் 08, 2022] Lars Ingebrigtsen wrote:

> Philip Kaludercic <philipk@posteo.net> writes:
>
>> Generally speaking, the ability to set a variable only if the symbol is
>> bound would be useful for features like these.
>
> Yes, that would be very nice.  But I think introducing something like
> that would be difficult...
>
> Introduce a new syntax like
>
> ;; Local Variables:
> ;; foo-bar: t boundp
> ;; End:
>
> is difficult to make backwards-compatible, but perhaps we can come up
> with something that older Emacsen will ignore but newer ones can parse?

Can we not change where boundp is located?  I.e.,

;; boundp foo-bar: t

?  IMO, this also reads nicely.

bug#55039: [PATCH] Use VC-relative file names in ChangeLog entries

[Lars Ingebrigtsen]

Visuwesh <visuweshm@gmail.com> writes:

> Can we not change where boundp is located?  I.e.,
>
> ;; boundp foo-bar: t
>
> ?  IMO, this also reads nicely.

But is that backwards-compatible?

bug#55039: [PATCH] Use VC-relative file names in ChangeLog entries

[Philip Kaludercic]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Visuwesh <visuweshm@gmail.com> writes:
>
>> Can we not change where boundp is located?  I.e.,
>>
>> ;; boundp foo-bar: t
>>
>> ?  IMO, this also reads nicely.
>
> But is that backwards-compatible?
                        ^ typo?

If you meant incompatible, it is any more or less incompatible than
adding the boundp to the end of the line?

bug#55039: [PATCH] Use VC-relative file names in ChangeLog entries

[Lars Ingebrigtsen]

Philip Kaludercic <philipk@posteo.net> writes:

>> But is that backwards-compatible?
>                         ^ typo?

No?

> If you meant incompatible, it is any more or less incompatible than
> adding the boundp to the end of the line?

It was a question -- I wouldn't have asked if I knew what the answer
was.

bug#55039: [PATCH] Use VC-relative file names in ChangeLog entries

[Philip Kaludercic]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Philip Kaludercic <philipk@posteo.net> writes:
>
>>> But is that backwards-compatible?
>>                         ^ typo?
>
> No?
>
>> If you meant incompatible, it is any more or less incompatible than
>> adding the boundp to the end of the line?
>
> It was a question -- I wouldn't have asked if I knew what the answer
> was.

In that case I am confused, if

;; boundp foo-bar: t

is backwards-compatible it appears preferable.

bug#55039: [PATCH] Use VC-relative file names in ChangeLog entries

[Lars Ingebrigtsen]

Philip Kaludercic <philipk@posteo.net> writes:

> In that case I am confused, if
>
> ;; boundp foo-bar: t
>
> is backwards-compatible it appears preferable.

Now I'm confused!  I'm not sure how you can be confused by a request for
information?

bug#55039: [PATCH] Use VC-relative file names in ChangeLog entries

[Philip Kaludercic]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Philip Kaludercic <philipk@posteo.net> writes:
>
>> In that case I am confused, if
>>
>> ;; boundp foo-bar: t
>>
>> is backwards-compatible it appears preferable.
>
> Now I'm confused!  I'm not sure how you can be confused by a request for
> information?

Never mind, I had misread the thread.  It appears that if
backwards-compatibility is of importance, an additional section would be
necessary.  Perhaps this could also be used as an opportunity to update
how buffer local variables are specified in general.  Are there any
other issues that are known besides this one?

Robert Pluim <rpluim@gmail.com> writes:

>>>>>> On Thu, 08 Sep 2022 12:53:07 +0000, Philip Kaludercic <philipk@posteo.net> said:
>
>     Philip> Lars Ingebrigtsen <larsi@gnus.org> writes:
>     >> Philip Kaludercic <philipk@posteo.net> writes:
>     >> 
>     >>>> But is that backwards-compatible?
>     >>> ^ typo?
>     >> 
>     >> No?
>     >> 
>     >>> If you meant incompatible, it is any more or less incompatible than
>     >>> adding the boundp to the end of the line?
>     >> 
>     >> It was a question -- I wouldn't have asked if I knew what the answer
>     >> was.
>
>     Philip> In that case I am confused, if
>
>     Philip> ;; boundp foo-bar: t
>
>     Philip> is backwards-compatible it appears preferable.
>
> File mode specification error: (user-error Malformed local variable line: "boundp foo-bar: t")
>
> Robert

bug#55039: [PATCH] Use VC-relative file names in ChangeLog entries

[Lars Ingebrigtsen]

Philip Kaludercic <philipk@posteo.net> writes:

> Perhaps this could also be used as an opportunity to update how buffer
> local variables are specified in general.  Are there any other issues
> that are known besides this one?

There was also discussion about whether to allow some variables to be
set before calling the mode function, and not afterwards.  Major modes
clear local variables, so they have to be set afterwards.  But some
major modes behave differently based on variable settings, so they
should be set first sometimes.

Being able to specify that would also be nice.

bug#55039: [PATCH] Use VC-relative file names in ChangeLog entries

[Robert Pluim]

>>>>> On Thu, 08 Sep 2022 12:53:07 +0000, Philip Kaludercic <philipk@posteo.net> said:

    Philip> Lars Ingebrigtsen <larsi@gnus.org> writes:
    >> Philip Kaludercic <philipk@posteo.net> writes:
    >> 
    >>>> But is that backwards-compatible?
    >>> ^ typo?
    >> 
    >> No?
    >> 
    >>> If you meant incompatible, it is any more or less incompatible than
    >>> adding the boundp to the end of the line?
    >> 
    >> It was a question -- I wouldn't have asked if I knew what the answer
    >> was.

    Philip> In that case I am confused, if

    Philip> ;; boundp foo-bar: t

    Philip> is backwards-compatible it appears preferable.

File mode specification error: (user-error Malformed local variable line: "boundp foo-bar: t")

Robert
-- 

bug#55039: [PATCH] Use VC-relative file names in ChangeLog entries

[Lars Ingebrigtsen]

Robert Pluim <rpluim@gmail.com> writes:

> File mode specification error: (user-error Malformed local variable
> line: "boundp foo-bar: t")

Thanks.

bug#55039: [PATCH] Use VC-relative file names in ChangeLog entries

[Visuwesh]

[வியாழன் செப்டம்பர் 08, 2022] Lars Ingebrigtsen wrote:

> Visuwesh <visuweshm@gmail.com> writes:
>
>> Can we not change where boundp is located?  I.e.,
>>
>> ;; boundp foo-bar: t
>>
>> ?  IMO, this also reads nicely.
>
> But is that backwards-compatible?

Doesn't seem to be the case unfortunately.  No idea what I was thinking
about when I wrote that.

bug#55039: [PATCH] Use VC-relative file names in ChangeLog entries

[Robert Pluim]

>>>>> On Thu, 08 Sep 2022 17:49:19 +0530, Visuwesh <visuweshm@gmail.com> said:

    Visuwesh> [வியாழன் செப்டம்பர் 08, 2022] Lars Ingebrigtsen wrote:
    >> Philip Kaludercic <philipk@posteo.net> writes:
    >> 
    >>> Generally speaking, the ability to set a variable only if the symbol is
    >>> bound would be useful for features like these.
    >> 
    >> Yes, that would be very nice.  But I think introducing something like
    >> that would be difficult...
    >> 
    >> Introduce a new syntax like
    >> 
    >> ;; Local Variables:
    >> ;; foo-bar: t boundp
    >> ;; End:
    >> 
    >> is difficult to make backwards-compatible, but perhaps we can come up
    >> with something that older Emacsen will ignore but newer ones can parse?

    Visuwesh> Can we not change where boundp is located?  I.e.,

    Visuwesh> ;; boundp foo-bar: t

    Visuwesh> ?  IMO, this also reads nicely.

Thereʼs always:

eval: (when (boundp ʼfoo-bar) (setq foo-bar t))

but that causes prompting by default.

Robert
-- 

bug#55039: [PATCH] Use VC-relative file names in ChangeLog entries

[Juri Linkov]

> +(defcustom diff-relative-names-in-changelog nil
> +  "Use relative file names when generating ChangeLog messages."
> +  :type 'boolean
> +  :safe #'booleanp
> +  :version "29.1")
> +
>  (defun diff-add-log-current-defuns ()
>    "Return an alist of defun names for the current diff.
>  The elements of the alist are of the form (FILE . (DEFUN...)),

Couldn't they share the same name prefix, e.g.
diff-add-log-relative-names?

bug#55039: [PATCH] Use VC-relative file names in ChangeLog entries

[Philip Kaludercic]

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


Here is an updated patch:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Add-new-user-option-diff-add-log-relative-names.patch --]
[-- Type: text/x-diff, Size: 4270 bytes --]

From f8e4b14198e8a9ccd5e4cbdbe044eb468697c2b8 Mon Sep 17 00:00:00 2001
From: Philip Kaludercic <philipk@posteo.net>
Date: Tue, 6 Sep 2022 22:06:29 +0200
Subject: [PATCH] Add new user option 'diff-add-log-relative-names'

* .dir-locals.el: Set 'diff-add-log-relative-names' to t
* doc/emacs/maintaining.texi (Log Buffer): Mention
'diff-add-log-relative-names'
* etc/NEWS: Mention 'diff-add-log-relative-names'
* lisp/vc/diff-mode.el (diff-add-log-relative-names): Add new option.
(diff-add-log-current-defuns): Use new option.
---
 .dir-locals.el             |  3 ++-
 doc/emacs/maintaining.texi |  5 ++++-
 etc/NEWS                   |  5 +++++
 lisp/vc/diff-mode.el       | 17 +++++++++++++++--
 4 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/.dir-locals.el b/.dir-locals.el
index 7812beb001..3312021920 100644
--- a/.dir-locals.el
+++ b/.dir-locals.el
@@ -5,7 +5,8 @@
          (sentence-end-double-space . t)
          (fill-column . 70)
 	 (emacs-lisp-docstring-fill-column . 65)
-         (bug-reference-url-format . "https://debbugs.gnu.org/%s")))
+         (bug-reference-url-format . "https://debbugs.gnu.org/%s")
+	 (diff-add-log-relative-names . t)))
  (c-mode . ((c-file-style . "GNU")
             (c-noise-macro-names . ("INLINE" "ATTRIBUTE_NO_SANITIZE_UNDEFINED" "UNINIT" "CALLBACK" "ALIGN_STACK"))
             (electric-quote-comment . nil)
diff --git a/doc/emacs/maintaining.texi b/doc/emacs/maintaining.texi
index 60169d8d8c..0f1dc18f1a 100644
--- a/doc/emacs/maintaining.texi
+++ b/doc/emacs/maintaining.texi
@@ -694,7 +694,10 @@ Log Buffer
 (@code{log-edit-generate-changelog-from-diff}), to generate skeleton
 ChangeLog entries, listing all changed file and function names based
 on the diff of the VC fileset.  Consecutive entries left empty will be
-combined by @kbd{C-q} (@code{fill-paragraph}).
+combined by @kbd{C-q} (@code{fill-paragraph}).  By default the
+inserted file names will just be the files without a path.  If
+@code{diff-add-log-relative-names} is non-nil, a partial path relative
+to the VC root directory will be inserted instead.
 
 @kindex C-c C-a @r{(Log Edit mode)}
 @findex log-edit-insert-changelog
diff --git a/etc/NEWS b/etc/NEWS
index 1317cd0128..0547796b6c 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1288,6 +1288,11 @@ Sets the value of the buffer-local variable 'whitespace-style' in
 'diff-mode' buffers.  By default, this variable is '(face trailing)',
 which preserves behavior from previous Emacs versions.
 
++++
+*** New user option 'diff-relative-names-in-changelog'.
+If non-nil insert file names in ChangeLog skeletons relative to the
+VC root directory.
+
 ** Ispell
 
 ---
diff --git a/lisp/vc/diff-mode.el b/lisp/vc/diff-mode.el
index 6b30de3cb3..7cafb43334 100644
--- a/lisp/vc/diff-mode.el
+++ b/lisp/vc/diff-mode.el
@@ -2336,10 +2336,18 @@ diff-undo
   (let ((inhibit-read-only t))
     (undo arg)))
 
+(defcustom diff-add-log-relative-names nil
+  "Use relative file names when generating ChangeLog messages."
+  :type 'boolean
+  :safe #'booleanp
+  :version "29.1")
+
 (defun diff-add-log-current-defuns ()
   "Return an alist of defun names for the current diff.
 The elements of the alist are of the form (FILE . (DEFUN...)),
-where DEFUN... is a list of function names found in FILE."
+where DEFUN... is a list of function names found in FILE.  If
+`diff-add-log-relative-names' is non-nil, insert file names
+relative to the VC root directory."
   (save-excursion
     (goto-char (point-min))
     (let* ((defuns nil)
@@ -2373,7 +2381,12 @@ diff-add-log-current-defuns
           ;; hunks (e.g., "diff --git ..." etc).
           (re-search-forward diff-hunk-header-re nil t)
         (setq hunk-end (save-excursion (diff-end-of-hunk)))
-        (pcase-let* ((filename (substring-no-properties (diff-find-file-name)))
+        (pcase-let* ((filename (substring-no-properties
+                                (if diff-add-log-relative-names
+                                    (file-relative-name
+                                     (diff-find-file-name)
+                                     (vc-root-dir))
+                                  (diff-find-file-name))))
                      (=lines 0)
                      (+lines 0)
                      (-lines 0)
-- 
2.30.2

bug#55039: [PATCH] Use VC-relative file names in ChangeLog entries

[Robert Pluim]

>>>>> On Tue, 06 Sep 2022 20:10:41 +0000, Philip Kaludercic <philipk@posteo.net> said:

    Philip> Here is an updated patch:

    Philip> From f8e4b14198e8a9ccd5e4cbdbe044eb468697c2b8 Mon Sep 17 00:00:00 2001
    Philip> From: Philip Kaludercic <philipk@posteo.net>
    Philip> Date: Tue, 6 Sep 2022 22:06:29 +0200
    Philip> Subject: [PATCH] Add new user option 'diff-add-log-relative-names'

    Philip> * .dir-locals.el: Set 'diff-add-log-relative-names' to t
    Philip> * doc/emacs/maintaining.texi (Log Buffer): Mention
    Philip> 'diff-add-log-relative-names'
    Philip> * etc/NEWS: Mention 'diff-add-log-relative-names'
    Philip> * lisp/vc/diff-mode.el (diff-add-log-relative-names): Add new option.
    Philip> (diff-add-log-current-defuns): Use new option.

The name of the option in NEWS is different from whatʼs in the code (I
prefer the one in NEWS).

Robert
-- 

bug#55039: [PATCH] Use VC-relative file names in ChangeLog entries

[Lars Ingebrigtsen]

Philip Kaludercic <philipk@posteo.net> writes:

> Here is an updated patch:

Looks good to me.

bug#55039: [PATCH] Use VC-relative file names in ChangeLog entries

[Eli Zaretskii]

> From: Philip Kaludercic <philipk@posteo.net>
> Cc: Eli Zaretskii <eliz@gnu.org>,  55039@debbugs.gnu.org
> Date: Tue, 06 Sep 2022 20:10:41 +0000
> 
> --- a/doc/emacs/maintaining.texi
> +++ b/doc/emacs/maintaining.texi
> @@ -694,7 +694,10 @@ Log Buffer
>  (@code{log-edit-generate-changelog-from-diff}), to generate skeleton
>  ChangeLog entries, listing all changed file and function names based
>  on the diff of the VC fileset.  Consecutive entries left empty will be
> -combined by @kbd{C-q} (@code{fill-paragraph}).
> +combined by @kbd{C-q} (@code{fill-paragraph}).  By default the
> +inserted file names will just be the files without a path.  If
> +@code{diff-add-log-relative-names} is non-nil, a partial path relative
> +to the VC root directory will be inserted instead.

GNU Coding Standards frown on using "path" for anything but PATH-style
directory lists.  Please use "leading directories" or somesuch here,
as that's what you mean.

> +*** New user option 'diff-relative-names-in-changelog'.
> +If non-nil insert file names in ChangeLog skeletons relative to the
             ^
Comma missing there.

> +(defcustom diff-add-log-relative-names nil
> +  "Use relative file names when generating ChangeLog messages."

This doesn't say relative to what.  It also doesn't mention the
commands which are affected.

>  (defun diff-add-log-current-defuns ()
>    "Return an alist of defun names for the current diff.
>  The elements of the alist are of the form (FILE . (DEFUN...)),
> -where DEFUN... is a list of function names found in FILE."
> +where DEFUN... is a list of function names found in FILE.  If
> +`diff-add-log-relative-names' is non-nil, insert file names
> +relative to the VC root directory."

The "insert" part is out of place here, since there's no insertion.

How about

  If `diff-add-log-relative-names' is non-nil, file names in the alist
  are relative to the root directory of the VC repository.

instead?

bug#55039: [PATCH] Use VC-relative file names in ChangeLog entries

[Philip Kaludercic]

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

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Philip Kaludercic <philipk@posteo.net>
>> Cc: Eli Zaretskii <eliz@gnu.org>,  55039@debbugs.gnu.org
>> Date: Tue, 06 Sep 2022 20:10:41 +0000
>> 
>> --- a/doc/emacs/maintaining.texi
>> +++ b/doc/emacs/maintaining.texi
>> @@ -694,7 +694,10 @@ Log Buffer
>>  (@code{log-edit-generate-changelog-from-diff}), to generate skeleton
>>  ChangeLog entries, listing all changed file and function names based
>>  on the diff of the VC fileset.  Consecutive entries left empty will be
>> -combined by @kbd{C-q} (@code{fill-paragraph}).
>> +combined by @kbd{C-q} (@code{fill-paragraph}).  By default the
>> +inserted file names will just be the files without a path.  If
>> +@code{diff-add-log-relative-names} is non-nil, a partial path relative
>> +to the VC root directory will be inserted instead.
>
> GNU Coding Standards frown on using "path" for anything but PATH-style
> directory lists.  Please use "leading directories" or somesuch here,
> as that's what you mean.

I've just remove the changes to the manual from my patch.

>> +*** New user option 'diff-relative-names-in-changelog'.
>> +If non-nil insert file names in ChangeLog skeletons relative to the
>              ^
> Comma missing there.

Done.

>> +(defcustom diff-add-log-relative-names nil
>> +  "Use relative file names when generating ChangeLog messages."
>
> This doesn't say relative to what.  It also doesn't mention the
> commands which are affected.

Added.

>>  (defun diff-add-log-current-defuns ()
>>    "Return an alist of defun names for the current diff.
>>  The elements of the alist are of the form (FILE . (DEFUN...)),
>> -where DEFUN... is a list of function names found in FILE."
>> +where DEFUN... is a list of function names found in FILE.  If
>> +`diff-add-log-relative-names' is non-nil, insert file names
>> +relative to the VC root directory."
>
> The "insert" part is out of place here, since there's no insertion.
>
> How about
>
>   If `diff-add-log-relative-names' is non-nil, file names in the alist
>   are relative to the root directory of the VC repository.
>
> instead?

I like it.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Add-new-user-option-diff-add-log-use-relative-names.patch --]
[-- Type: text/x-diff, Size: 3545 bytes --]

From 8ef441e5d5340b06dd04d294a384ae05bf7f5615 Mon Sep 17 00:00:00 2001
From: Philip Kaludercic <philipk@posteo.net>
Date: Tue, 6 Sep 2022 22:06:29 +0200
Subject: [PATCH] Add new user option 'diff-add-log-use-relative-names'

* .dir-locals.el: Set 'diff-add-log-use-relative-names' to t
* etc/NEWS: Mention 'diff-add-log-relative-names'
* lisp/vc/diff-mode.el (diff-add-log-use-relative-names): Add new option.
(diff-add-log-current-defuns): Use new option.
---
 .dir-locals.el       |  3 ++-
 etc/NEWS             |  5 +++++
 lisp/vc/diff-mode.el | 20 ++++++++++++++++++--
 3 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/.dir-locals.el b/.dir-locals.el
index 1c90ddcf56..9882a19f85 100644
--- a/.dir-locals.el
+++ b/.dir-locals.el
@@ -5,7 +5,8 @@
          (sentence-end-double-space . t)
          (fill-column . 70)
 	 (emacs-lisp-docstring-fill-column . 65)
-         (bug-reference-url-format . "https://debbugs.gnu.org/%s")))
+         (bug-reference-url-format . "https://debbugs.gnu.org/%s")
+	 (diff-add-log-use-relative-names . t)))
  (c-mode . ((c-file-style . "GNU")
             (c-noise-macro-names . ("INLINE" "ATTRIBUTE_NO_SANITIZE_UNDEFINED" "UNINIT" "CALLBACK" "ALIGN_STACK"))
             (electric-quote-comment . nil)
diff --git a/etc/NEWS b/etc/NEWS
index b61b88d6fb..76c66a8e39 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1340,6 +1340,11 @@ Sets the value of the buffer-local variable 'whitespace-style' in
 'diff-mode' buffers.  By default, this variable is '(face trailing)',
 which preserves behavior from previous Emacs versions.
 
++++
+*** New user option 'diff-add-log-use-relative-names'.
+If non-nil insert file names in ChangeLog skeletons relative to the
+VC root directory.
+
 ** Ispell
 
 ---
diff --git a/lisp/vc/diff-mode.el b/lisp/vc/diff-mode.el
index a01943437c..1d2fbca0e5 100644
--- a/lisp/vc/diff-mode.el
+++ b/lisp/vc/diff-mode.el
@@ -2336,10 +2336,21 @@ diff-undo
   (let ((inhibit-read-only t))
     (undo arg)))
 
+(defcustom diff-add-log-use-relative-names nil
+  "Use relative file names when generating ChangeLog skeletons.
+The files will be relative to the root directory of the VC
+repository.  This option affects the behaviour of
+`diff-add-log-current-defuns'."
+  :type 'boolean
+  :safe #'booleanp
+  :version "29.1")
+
 (defun diff-add-log-current-defuns ()
   "Return an alist of defun names for the current diff.
 The elements of the alist are of the form (FILE . (DEFUN...)),
-where DEFUN... is a list of function names found in FILE."
+where DEFUN... is a list of function names found in FILE.  If
+`diff-add-log-use-relative-names' is non-nil, file names in the alist
+are relative to the root directory of the VC repository."
   (save-excursion
     (goto-char (point-min))
     (let* ((defuns nil)
@@ -2373,7 +2384,12 @@ diff-add-log-current-defuns
           ;; hunks (e.g., "diff --git ..." etc).
           (re-search-forward diff-hunk-header-re nil t)
         (setq hunk-end (save-excursion (diff-end-of-hunk)))
-        (pcase-let* ((filename (substring-no-properties (diff-find-file-name)))
+        (pcase-let* ((filename (substring-no-properties
+                                (if diff-add-log-use-relative-names
+                                    (file-relative-name
+                                     (diff-find-file-name)
+                                     (vc-root-dir))
+                                  (diff-find-file-name))))
                      (=lines 0)
                      (+lines 0)
                      (-lines 0)
-- 
2.30.2

bug#55039: [PATCH] Use VC-relative file names in ChangeLog entries

[Eli Zaretskii]

> From: Philip Kaludercic <philipk@posteo.net>
> Cc: larsi@gnus.org,  55039@debbugs.gnu.org
> Date: Wed, 07 Sep 2022 15:53:29 +0000
> 
> >> --- a/doc/emacs/maintaining.texi
> >> +++ b/doc/emacs/maintaining.texi
> >> @@ -694,7 +694,10 @@ Log Buffer
> >>  (@code{log-edit-generate-changelog-from-diff}), to generate skeleton
> >>  ChangeLog entries, listing all changed file and function names based
> >>  on the diff of the VC fileset.  Consecutive entries left empty will be
> >> -combined by @kbd{C-q} (@code{fill-paragraph}).
> >> +combined by @kbd{C-q} (@code{fill-paragraph}).  By default the
> >> +inserted file names will just be the files without a path.  If
> >> +@code{diff-add-log-relative-names} is non-nil, a partial path relative
> >> +to the VC root directory will be inserted instead.
> >
> > GNU Coding Standards frown on using "path" for anything but PATH-style
> > directory lists.  Please use "leading directories" or somesuch here,
> > as that's what you mean.
> 
> I've just remove the changes to the manual from my patch.

Why?  That was nowhere near what I intended to say with the above
comments, which were minor.

bug#55039: [PATCH] Use VC-relative file names in ChangeLog entries

[Philip Kaludercic]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Philip Kaludercic <philipk@posteo.net>
>> Cc: larsi@gnus.org,  55039@debbugs.gnu.org
>> Date: Wed, 07 Sep 2022 15:53:29 +0000
>> 
>> >> --- a/doc/emacs/maintaining.texi
>> >> +++ b/doc/emacs/maintaining.texi
>> >> @@ -694,7 +694,10 @@ Log Buffer
>> >>  (@code{log-edit-generate-changelog-from-diff}), to generate skeleton
>> >>  ChangeLog entries, listing all changed file and function names based
>> >>  on the diff of the VC fileset.  Consecutive entries left empty will be
>> >> -combined by @kbd{C-q} (@code{fill-paragraph}).
>> >> +combined by @kbd{C-q} (@code{fill-paragraph}).  By default the
>> >> +inserted file names will just be the files without a path.  If
>> >> +@code{diff-add-log-relative-names} is non-nil, a partial path relative
>> >> +to the VC root directory will be inserted instead.
>> >
>> > GNU Coding Standards frown on using "path" for anything but PATH-style
>> > directory lists.  Please use "leading directories" or somesuch here,
>> > as that's what you mean.
>> 
>> I've just remove the changes to the manual from my patch.
>
> Why?  That was nowhere near what I intended to say with the above
> comments, which were minor.

It wasn't because of your comments, rather I just felt mentioning it was
too clumsy, and a few others said they didn't feel it was necessary to
mention the option in the manual.  But if you think that it should be
added, then I'll try it again and take your comments into account.

bug#55039: [PATCH] Use VC-relative file names in ChangeLog entries

[Eli Zaretskii]

> From: Philip Kaludercic <philipk@posteo.net>
> Cc: larsi@gnus.org,  55039@debbugs.gnu.org
> Date: Wed, 07 Sep 2022 18:27:18 +0000
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> I've just remove the changes to the manual from my patch.
> >
> > Why?  That was nowhere near what I intended to say with the above
> > comments, which were minor.
> 
> It wasn't because of your comments, rather I just felt mentioning it was
> too clumsy, and a few others said they didn't feel it was necessary to
> mention the option in the manual.  But if you think that it should be
> added, then I'll try it again and take your comments into account.

Yes, I think it should be in the manual.  But if you'd rather leave
that to me, I'm fine with that.

bug#55039: [PATCH] Use VC-relative file names in ChangeLog entries

[Philip Kaludercic]

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

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Philip Kaludercic <philipk@posteo.net>
>> Cc: larsi@gnus.org,  55039@debbugs.gnu.org
>> Date: Wed, 07 Sep 2022 18:27:18 +0000
>> 
>> Eli Zaretskii <eliz@gnu.org> writes:
>> 
>> >> I've just remove the changes to the manual from my patch.
>> >
>> > Why?  That was nowhere near what I intended to say with the above
>> > comments, which were minor.
>> 
>> It wasn't because of your comments, rather I just felt mentioning it was
>> too clumsy, and a few others said they didn't feel it was necessary to
>> mention the option in the manual.  But if you think that it should be
>> added, then I'll try it again and take your comments into account.
>
> Yes, I think it should be in the manual.  But if you'd rather leave
> that to me, I'm fine with that.

Here is my latest attempt (will merge it into the patch):


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

diff --git a/doc/emacs/maintaining.texi b/doc/emacs/maintaining.texi
index 343cc83ce5..f16d0a922e 100644
--- a/doc/emacs/maintaining.texi
+++ b/doc/emacs/maintaining.texi
@@ -694,7 +694,10 @@ Log Buffer
 (@code{log-edit-generate-changelog-from-diff}), to generate skeleton
 ChangeLog entries, listing all changed file and function names based
 on the diff of the VC fileset.  Consecutive entries left empty will be
-combined by @kbd{C-q} (@code{fill-paragraph}).
+combined by @kbd{C-q} (@code{fill-paragraph}).  By default the
+skeleton will just include the file name, without any leading
+directories.  If you wish to prepend the leading directories up to the
+VC root, customize @code{diff-add-log-use-relative-names}.
 
 @kindex C-c C-a @r{(Log Edit mode)}
 @findex log-edit-insert-changelog

bug#55039: [PATCH] Use VC-relative file names in ChangeLog entries

[Eli Zaretskii]

> From: Philip Kaludercic <philipk@posteo.net>
> Cc: larsi@gnus.org,  55039@debbugs.gnu.org
> Date: Wed, 07 Sep 2022 21:23:38 +0000
> 
> >> It wasn't because of your comments, rather I just felt mentioning it was
> >> too clumsy, and a few others said they didn't feel it was necessary to
> >> mention the option in the manual.  But if you think that it should be
> >> added, then I'll try it again and take your comments into account.
> >
> > Yes, I think it should be in the manual.  But if you'd rather leave
> > that to me, I'm fine with that.
> 
> Here is my latest attempt (will merge it into the patch):

LGTM, thanks.

bug#55039: [PATCH] Use VC-relative file names in ChangeLog entries

[Philip Kaludercic]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Philip Kaludercic <philipk@posteo.net>
>> Cc: larsi@gnus.org,  55039@debbugs.gnu.org
>> Date: Wed, 07 Sep 2022 21:23:38 +0000
>> 
>> >> It wasn't because of your comments, rather I just felt mentioning it was
>> >> too clumsy, and a few others said they didn't feel it was necessary to
>> >> mention the option in the manual.  But if you think that it should be
>> >> added, then I'll try it again and take your comments into account.
>> >
>> > Yes, I think it should be in the manual.  But if you'd rather leave
>> > that to me, I'm fine with that.
>> 
>> Here is my latest attempt (will merge it into the patch):
>
> LGTM, thanks.

Great, I've pushed the commit.

bug#55039: [PATCH] Use VC-relative file names in ChangeLog entries

[Richard Stallman]

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

  >   If
  > > +@code{diff-add-log-relative-names} is non-nil, a partial path relative
  > > +to the VC root directory will be inserted instead.

1. Please try to rewrite passive voice sentences using active voice.
Passive voice omits the subject and that tends to reduce clarity.

2. Shouldn't there be @code around `nil' in a Texinfo file?

-- 
Dr Richard Stallman (https://stallman.org)
Chief GNUisance of the GNU Project (https://gnu.org)
Founder, Free Software Foundation (https://fsf.org)
Internet Hall-of-Famer (https://internethalloffame.org)

bug#55039: [PATCH] Use VC-relative file names in ChangeLog entries

[Philip Kaludercic]

Richard Stallman <rms@gnu.org> writes:

> [[[ To any NSA and FBI agents reading my email: please consider    ]]]
> [[[ whether defending the US Constitution against all enemies,     ]]]
> [[[ foreign or domestic, requires you to follow Snowden's example. ]]]
>
>   >   If
>   > > +@code{diff-add-log-relative-names} is non-nil, a partial path relative
>   > > +to the VC root directory will be inserted instead.
>
> 1. Please try to rewrite passive voice sentences using active voice.
> Passive voice omits the subject and that tends to reduce clarity.
>
> 2. Shouldn't there be @code around `nil' in a Texinfo file?

You are right, but the patch you were commenting on was outdated.

bug#55039: [PATCH] Use VC-relative file names in ChangeLog entries

[Richard Stallman]

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

We used to have a ChangeLog file for each important directory
in Emacs (src, lisp, etc, lib-src, and so on).
I hope we still do, as a single ChangeLog for all of Emacs
would be very long and thus cumbersome.

In these ChangeLog files, we should delete the directory names which
are superfluous.  Thus, when generating src/ChangeLog, we should
delete `src/' from the start of the source file names in the entries.

Do we do that?

Would this patch require changes in that?


-- 
Dr Richard Stallman (https://stallman.org)
Chief GNUisance of the GNU Project (https://gnu.org)
Founder, Free Software Foundation (https://fsf.org)
Internet Hall-of-Famer (https://internethalloffame.org)

bug#55039: [PATCH] Use VC-relative file names in ChangeLog entries

[Eli Zaretskii]

> From: Richard Stallman <rms@gnu.org>
> Date: Thu, 21 Apr 2022 23:03:32 -0400
> Cc: 55039@debbugs.gnu.org
> 
> We used to have a ChangeLog file for each important directory
> in Emacs (src, lisp, etc, lib-src, and so on).
> I hope we still do, as a single ChangeLog for all of Emacs
> would be very long and thus cumbersome.

We nowadays have a single ChangeLog for all of Emacs, which is
produced from the Git commit logs.  We start a new ChangeLog from time
to time, to keep the file from growing too much, but that's all.

bug#55039: [PATCH] Use VC-relative file names in ChangeLog entries

[Richard Stallman]

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

  > We nowadays have a single ChangeLog for all of Emacs, which is
  > produced from the Git commit logs.  We start a new ChangeLog from time
  > to time, to keep the file from growing too much, but that's all.

Do you find it inconvenient to put all directories into a single
ChangeLog file?  I would expect it to be inconvenient, but I don't
have actual experience using these files.

-- 
Dr Richard Stallman (https://stallman.org)
Chief GNUisance of the GNU Project (https://gnu.org)
Founder, Free Software Foundation (https://fsf.org)
Internet Hall-of-Famer (https://internethalloffame.org)

bug#55039: [PATCH] Use VC-relative file names in ChangeLog entries

[Eli Zaretskii]

> From: Richard Stallman <rms@gnu.org>
> Cc: philipk@posteo.net, 55039@debbugs.gnu.org
> Date: Fri, 22 Apr 2022 23:46:27 -0400
> 
>   > We nowadays have a single ChangeLog for all of Emacs, which is
>   > produced from the Git commit logs.  We start a new ChangeLog from time
>   > to time, to keep the file from growing too much, but that's all.
> 
> Do you find it inconvenient to put all directories into a single
> ChangeLog file?

No, not really.  We do that now for a long time, so I'm used to it
nowadays.