Subject: Re: bug#64055: [WIP Patch] Enable editing commit messages - vc-git-modify-change-comment

From: Sean Whitton <[email-masked]>
Date: Fri, 18 Oct 2024 12:50:18 +0800

Hello,

On Fri 18 Oct 2024 at 03:46am +03, Dmitry Gutov wrote:

> Hi! Sorry for the late reply, I've been driving a lot the last week.

Oh, no problem.

> On 10/10/2024 05:39, Sean Whitton wrote:
>> In Dmitry's patch he takes the approach of calling the
>> expanded-log-entry backend function to get the message to edit.
>> This is not a real VC backend function -- in fact it's a log-view
>> feature, log-view-expanded-log-entry-function.
>
> It is, but it is also basically a backend method, i.e. in vc-git.el:
>
>     (setq-local log-view-expanded-log-entry-function
>                 'vc-git-expanded-log-entry))
>
> So if we determined that its output could be used for editing, perhaps after
> some massaging (e.g. reindenting and keeping only a subset of the headers),
> that could be a minor win -- fewer methods is better in general. I haven't
> tried to code it, so there could be pitfalls.

I thought about it and realised that, for the git case, variables like
vc-git-log-switches and vc-git-shortlog-switches can affect the output,
and could make us misparse it.  And ultimately the benefits of avoiding
a new backend method didn't seem to outweigh having parsing code that
could turn out to be fragile.

> Also note that we have 'rfc822-goto-eoh' which can be used to skip to the end
> of the headers. 'log-edit-extract-headers' could be used as reference for
> extracting "Summary", even if it doesn't exactly give us the desired info now.

Those aren't VC-specific, though, they're based on how Log Edit works
for all backends.  So although it's fiddly parsing, it can be done the
same way always, at least.

> The new vc-git-get-change-comment seems good in terms of functionality. I was
> thinking that the headers such as Author, No-Verify and Sign-Off, might be
> good to show as well, but as long as their values are intact after the edit,
> that's optional.

I thought that it would be nice to include Author, in particular, as you
might need to amend the value.  That would require adding additional
arguments to the modify-change-comment action, though, so I left it for
later if someone wants to implement it.

I'm not sure how No-Verify would interact with the --fixup=reword:
commits we are now planning to use.

Sign-Off would be useful for using the new feature to insert a missing
sign-off.

>> I think, though, that there might be subtle complexities there.  For
>> example, should there be a FILES argument, or just a REVISION argument?
>> For Git and Hg it's just REVISION, but we wouldn't want to bake that in.
>
> Makes sense, e.g. if we end up supporting per-file backends later.

Right.

-- 
Sean Whitton