Re: [Emacs-diffs] emacs-25 5152b21 1/2: ; ChangeLog fixes

Re: [Emacs-diffs] emacs-25 5152b21 1/2: ; ChangeLog fixes

[Dmitry Gutov]

On 01/24/2016 03:32 PM, Nicolas Petton wrote:
> branch: emacs-25
> commit 5152b211e81f969ac7cf31e7be6564a8be3299c5
> Author: Nicolas Petton <nicolas@petton.fr>
> Commit: Nicolas Petton <nicolas@petton.fr>
>
>      ; ChangeLog fixes

Thanks!

> -	* nsterm.m (windowDidResize:, toggleFullScreen:):
> +	* src/nsterm.m (windowDidResize:, toggleFullScreen:):

Any idea how we get this problem? Do people type out file names 
manually, instead of using C-x 4 a?

Re: [Emacs-diffs] emacs-25 5152b21 1/2: ; ChangeLog fixes

[martin rudalics]

 > Any idea how we get this problem? Do people type out file names
 > manually, instead of using C-x 4 a?

I can make it happen by doing ‘add-change-log-entry-other-window’ from
another instance of Emacs than the one that gets changed.

martin

Re: [Emacs-diffs] emacs-25 5152b21 1/2: ; ChangeLog fixes

[Dmitry Gutov]

On 01/24/2016 05:38 PM, martin rudalics wrote:

> I can make it happen by doing ‘add-change-log-entry-other-window’ from
> another instance of Emacs than the one that gets changed.

Why does it matter? Does another instance run an older version of Emacs?

Re: [Emacs-diffs] emacs-25 5152b21 1/2: ; ChangeLog fixes

[martin rudalics]

 >> I can make it happen by doing ‘add-change-log-entry-other-window’ from
 >> another instance of Emacs than the one that gets changed.
 >
 > Why does it matter? Does another instance run an older version of Emacs?

I misread the diff.  When I do ‘add-change-log-entry-other-window’ for a
file in another Emacs instance, it inserts the path relative to the
source directory of the current instance.  So instead of something like

* src/nsterm.m (windowDidResize:, toggleFullScreen:):

I get, for example, something like

* ../trunk/src/nsterm.m (windowDidResize:, toggleFullScreen:):

in the ChangeLog file of my current instance.  Which is implicitly
correct but hardly useful for the commit message.

martin

Re: [Emacs-diffs] emacs-25 5152b21 1/2: ; ChangeLog fixes

[Dmitry Gutov]

On 01/24/2016 07:21 PM, martin rudalics wrote:

> I misread the diff.  When I do ‘add-change-log-entry-other-window’ for a
> file in another Emacs instance, it inserts the path relative to the
> source directory of the current instance.  So instead of something like
>
> * src/nsterm.m (windowDidResize:, toggleFullScreen:):
>
> I get, for example, something like
>
> * ../trunk/src/nsterm.m (windowDidResize:, toggleFullScreen:):
>
> in the ChangeLog file of my current instance.  Which is implicitly
> correct but hardly useful for the commit message.

Any idea how that happens? There are no references to source-directory 
inside lisp/vc/.

Maybe you've edited files in both branches (residing in different 
directories) in that session, and still keep the ChangeLog file from the 
other branch open?

Re: [Emacs-diffs] emacs-25 5152b21 1/2: ; ChangeLog fixes

[martin rudalics]

 > Any idea how that happens? There are no references to source-directory inside lisp/vc/.

Maybe some ‘default-directory’ vs ‘expand-file-name’ snafu.

 > Maybe you've edited files in both branches (residing in different
 > directories) in that session, and still keep the ChangeLog file from
 > the other branch open?

Possible but unlikely.

I have a special wrapper for ‘add-change-log-entry-other-window’ so I
can call it from the ediff control panel and it still uses the right
buffer.  I just would have to step through it with the debugger ...

martin

Re: [Emacs-diffs] emacs-25 5152b21 1/2: ; ChangeLog fixes

[Eli Zaretskii]

> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Sun, 24 Jan 2016 15:35:38 +0300
> 
> > -	* nsterm.m (windowDidResize:, toggleFullScreen:):
> > +	* src/nsterm.m (windowDidResize:, toggleFullScreen:):
> 
> Any idea how we get this problem? Do people type out file names 
> manually, instead of using C-x 4 a?

Most probably, yes.

Re: [Emacs-diffs] emacs-25 5152b21 1/2: ; ChangeLog fixes

[Dmitry Gutov]

On 01/24/2016 05:54 PM, Eli Zaretskii wrote:

>> Any idea how we get this problem? Do people type out file names
>> manually, instead of using C-x 4 a?
>
> Most probably, yes.

Maybe we could add another linter step to the pre-commit-hook that would 
check the existence of file names mentioned in the message.

That does sound more complex than what we're checking currently, but if 
we're serious about ChangeLog entries, having to review and fix the 
problems manually seems a bigger problem.

Re: [Emacs-diffs] emacs-25 5152b21 1/2: ; ChangeLog fixes

[Eli Zaretskii]

> Cc: emacs-devel@gnu.org, nicolas@petton.fr
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Sun, 24 Jan 2016 18:07:51 +0300
> 
> Maybe we could add another linter step to the pre-commit-hook that would 
> check the existence of file names mentioned in the message.

Could be a good idea, yes.

Re: [Emacs-diffs] emacs-25 5152b21 1/2: ; ChangeLog fixes

[Paul Eggert]

Dmitry Gutov wrote:
> Maybe we could add another linter step to the pre-commit-hook that would check
> the existence of file names mentioned in the message.

I typically use vc-dwim, which does that already. That is partly why I haven't 
bothered adding a separate linter step to build-aux/git-hooks/commit-msg. It'd 
be nice if the check were there for everybody.

vc-dwim also has the converse check. That is, it checks that every file affected 
by the commit is mentioned in the commit message. I prefer this check too, 
though I'm not sure everyone else would.

Re: [Emacs-diffs] emacs-25 5152b21 1/2: ; ChangeLog fixes

[Dmitry Gutov]

On 01/24/2016 10:52 PM, Paul Eggert wrote:

> I typically use vc-dwim, which does that already. That is partly why I
> haven't bothered adding a separate linter step to
> build-aux/git-hooks/commit-msg. It'd be nice if the check were there for
> everybody.

Hmm. I'm definitely not asking for this for my own benefit (you can see 
my log messages weren't in this list of fixes).

> vc-dwim also has the converse check. That is, it checks that every file
> affected by the commit is mentioned in the commit message. I prefer this
> check too, though I'm not sure everyone else would.

Why not? I think we have a clearly-defined set of files which can go 
without log entries. I'd say it's a question of policy and not just 
personal preference.

Re: [Emacs-diffs] emacs-25 5152b21 1/2: ; ChangeLog fixes

[Paul Eggert]

Dmitry Gutov wrote:
>> vc-dwim also has the converse check. That is, it checks that every file
>> affected by the commit is mentioned in the commit message. I prefer this
>> check too, though I'm not sure everyone else would.
>
> Why not? I think we have a clearly-defined set of files which can go without log
> entries.

vc-dwim doesn't know about that set, and so insists on every file being 
mentioned in the commit message. I prefer this style but the guidelines do not 
require it. But you're right, build-aux/git-hooks/commit-msg could know about 
that file set and complain only about the files not in the set.

Re: [Emacs-diffs] emacs-25 5152b21 1/2: ; ChangeLog fixes

[Stefan Monnier]

>> -	* nsterm.m (windowDidResize:, toggleFullScreen:):
>> +	* src/nsterm.m (windowDidResize:, toggleFullScreen:):
> Any idea how we get this problem? Do people type out file names manually,
> instead of using C-x 4 a?

Why do we care?  nsterm.m is unambiguous.


        Stefan

Re: [Emacs-diffs] emacs-25 5152b21 1/2: ; ChangeLog fixes

[Dmitry Gutov]

On 01/25/2016 05:21 AM, Stefan Monnier wrote:
>>> -	* nsterm.m (windowDidResize:, toggleFullScreen:):
>>> +	* src/nsterm.m (windowDidResize:, toggleFullScreen:):
>> Any idea how we get this problem? Do people type out file names manually,
>> instead of using C-x 4 a?
>
> Why do we care?  nsterm.m is unambiguous.

Wrong format?

Anyway, the same commit I was referring to also fixed actual problems in 
file names (wrong name, missing or wrong extension, etc).