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

[Dmitry Gutov]

Hi!

This is a welcome addition, thanks for your efforts.

On 14/06/2023 01:59, Morgan Smith wrote:
> -             "-1"  "--no-color" ,@(ensure-list vc-git-log-switches)
> +             "-1"  "--no-color"
> +             ;; The same as the default "medium" format but it doesn't
> +             ;; put spaces at the beginning of the body.  This is so
> +             ;; we can grab this as the initial value when calling
> +             ;; log-view-modify-change-comment
> +             "--pretty=format:commit %H%nAuthor: %an %ae%nDate:   %ad%n%n%B"
> +             ,@(ensure-list vc-git-log-switches)

Does this improve the overall look of the log? Otherwise, we could 
remove the extra spaces when grabbing the log message (as long as we 
know how many were added), just like we do in log-edit-insert-changelog.

But an even better solution, perhaps, would be to delegate to 
(vc-call-backend backend 'expanded-log-entry revision).

Here's how it looked in my old, unfinished version of this feature:

(defun log-view-modify-change-comment ()
  "Edit the change comment displayed at point."
  (interactive)
  (let* ((file (and log-view-per-file-logs
                    (log-view-current-file)))
         (revision (log-view-current-tag))
         (backend (vc-responsible-backend (or file (car 
log-view-vc-fileset)))))
    (vc-modify-change-comment (list file)
                              revision
                              (vc-call-backend backend 
'expanded-log-entry revision))))

It will need to fall back to log-view-extract-comment for backends that 
don't have this method defined, though. Such as CVS/RCS/SVN/SCCS.

> -;; - Third, doing message editing in log-view might be a natural way to go
> -;;   about it, but editing any but the last commit (and even it, if it's
> -;;   been pushed) is a dangerous operation in Git, which we shouldn't make
> -;;   too easy for users to perform.

...

> I found the hack located in vc-git-modify-change-comment' of setting
> "GIT_SEQUENCE_EDITOR=:" on the internet and I don't understand how it
> works.

Apparently ":' is a shell built-in which "does nothing and then 
succeeds" (https://stackoverflow.com/a/29094904). "true" should also work.

> Also I'm not really sure what the 'vc-dispatcher-browsing' function is
> supposed to do but it's getting in my way.  Like are 'diff-mode' buffers
> a "directory browser buffer"?

The docstring is 15 years old and never updated. If we can find a better 
wording, that would be great.

>  I don't understand the intent here.

Check out the two places where it is called.

> Also you'll noticed I deleted some comments in vc.el.  That's mainly to
> highlight them so we can talk about them now.  Is there a good way to
> stop users from editing remote commits?

We could do something like this: take the remote branch associated with 
the current branch, and its top commit. And then, somehow, see if the 
commit to be edited is reachable by traversing up history.

There likely are some known Git snippets out there on SO or general 
Internet that do these steps, but I haven't looked yet.

Anyway, we could do this check first thing inside 
vc-git-modify-change-comment and, when the operation is dangerous, 
doubly prompt the user whether they want to proceed.

Or, a more involved approach: add a new backend function which would do 
the check. Then log-view-modify-change-comment could abort earlier.

> I feel like I should probably just keep working on this and get it more
> polished and answer some of my own questions before posting here but I'm
> getting kind of frustrated with this.  I am many hours deep into this
> problem.

Please feel free to ask questions.