emacs-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: master 101f3cf5b9: Add support for user edits to VC command argument


From: Stefan Monnier
Subject: Re: master 101f3cf5b9: Add support for user edits to VC command arguments
Date: Fri, 23 Sep 2022 15:32:48 -0400
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/29.0.50 (gnu/linux)

>>> Previously it was inserted without going via the hook.
>> But why did you change the code so it goes via the hook?
> So that it uses the user-updated value of the command,

Ah, that makes sense.

> From b0fcba8791a1702a41df0d4f12bb47d151eec5b9 Mon Sep 17 00:00:00 2001
> From: Sean Whitton <spwhitton@spwhitton.name>
> Date: Fri, 23 Sep 2022 10:43:31 -0700
> Subject: [PATCH 1/2] vc-git--pushpull: Restore handling of vc-git-program
>
> * lisp/vc/vc-git.el (vc-git--pushpull): Restore handling of
> vc-git-program before recent change: respect a buffer-local value of
> vc-git-program, and don't ignore user edits to the git program name
> when PROMPT.

LGTM.

> From 594ff0b4bdad1f756614556f3487b98ad632c617 Mon Sep 17 00:00:00 2001
> From: Sean Whitton <spwhitton@spwhitton.name>
> Date: Fri, 23 Sep 2022 10:09:34 -0700
> Subject: [PATCH 2/2] Rework user edits to VC commands to use a more functional
>  style
>
> * lisp/vc/vc-dispatcher.el (vc-pre-command-functions): Delete.
> (vc-filter-command-function): New variable.
> (vc-user-edit-command): Factor out of vc-do-command.
> (vc-do-command, vc-do-async-command):
> * lisp/vc/vc-git.el (vc-git--pushpull):
> * lisp/vc/vc.el (vc-print-branch-log): Use vc-filter-command-function
> in place of vc-pre-command-functions and vc-pre-command-functions.

IIUC you could also remove the `vc-want-edit-command-p` variable, right?

> +(defvar vc-filter-command-function (lambda (&rest args) args)
> +  "Function called to transform VC commands before execution.
> +The function is called inside the buffer in which the command
> +will be run and is passed the COMMAND, FILE-OR-LIST and FLAGS
> +arguments to `vc-do-command'.  It should return a list with the
> +same structure containing new values for these arguments.")

That makes it sound like it receives 3 args (which sounds fine to me).

> +(defun vc-user-edit-command (command file-or-list &rest flags)

But these aren't 3 args, it's "2 or more".
So the doc and the code don't match.

BTW, `apply` and `&rest` are noticeably less efficient than `funcall`
and normal args.  Efficiency admittedly doesn't matter much here, but
I'd recommend we don't needlessly use apply/&rest (i.e. I recommend we
fix the code to match the doc than the other way around).


        Stefan




reply via email to

[Prev in Thread] Current Thread [Next in Thread]