From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Sean Whitton Newsgroups: gmane.emacs.devel Subject: Re: master 101f3cf5b9: Add support for user edits to VC command arguments Date: Thu, 22 Sep 2022 14:38:33 -0700 Message-ID: <87sfkjjdie.fsf@melete.silentflame.com> References: <166378878197.3568.7077863090259744101@vcs2.savannah.gnu.org> <20220921193303.1B687C11D81@vcs2.savannah.gnu.org> Mime-Version: 1.0 Content-Type: text/plain Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="20286"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/29.0.50 (gnu/linux) Cc: emacs-devel@gnu.org To: Stefan Monnier Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Thu Sep 22 23:40:01 2022 Return-path: Envelope-to: ged-emacs-devel@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1obTvM-00054r-Bh for ged-emacs-devel@m.gmane-mx.org; Thu, 22 Sep 2022 23:40:00 +0200 Original-Received: from localhost ([::1]:44906 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1obTvL-0003wG-D0 for ged-emacs-devel@m.gmane-mx.org; Thu, 22 Sep 2022 17:39:59 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:41486) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1obTu1-0002dC-LZ for emacs-devel@gnu.org; Thu, 22 Sep 2022 17:38:37 -0400 Original-Received: from out4-smtp.messagingengine.com ([66.111.4.28]:46155) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1obTtz-00057Y-Gl for emacs-devel@gnu.org; Thu, 22 Sep 2022 17:38:37 -0400 Original-Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailout.nyi.internal (Postfix) with ESMTP id 976715C00B0; Thu, 22 Sep 2022 17:38:34 -0400 (EDT) Original-Received: from mailfrontend1 ([10.202.2.162]) by compute1.internal (MEProxy); Thu, 22 Sep 2022 17:38:34 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=spwhitton.name; h=cc:cc:content-type:date:date:from:from:in-reply-to :in-reply-to:message-id:mime-version:references:reply-to:sender :subject:subject:to:to; s=fm1; t=1663882714; x=1663969114; bh=fj Rt9EkGO9/BaFpZRHUPXlBFhHue4c064M4lI7sYr14=; b=r6/hvQPXFmqH8wxy7b vSxYlmH3b5Cc5QxJajRyq4OohhcPGFtqiQNw1XSt5wTtj9Yi0Otyb1c31cXVql/z S3pgWxt5NbRXpS2sn3H064p2Qk9MmKsU2RpmTqMFHMajZ0oISwbRA4CjebD4NJCx Q4VJWb6vhsC2AMPQBukCl1ERpJ2gudqA8MUpuT+Ti93+fEKl56J8TJWlcwt1JqED BR/+Brr+EvPcMEc8DVwJnwKfS30AvOuVnfX6lOrtwkMDk7oc1P/suNGZlQIx09V4 PAJnlaYTbJr4qx+wdAIt/fM3ZKu4BN5gLx0VLugVoOQ1a529hsNJxSnT5qgMqFnO 89tw== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-type:date:date:feedback-id :feedback-id:from:from:in-reply-to:in-reply-to:message-id :mime-version:references:reply-to:sender:subject:subject:to:to :x-me-proxy:x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s= fm2; t=1663882714; x=1663969114; bh=fjRt9EkGO9/BaFpZRHUPXlBFhHue 4c064M4lI7sYr14=; b=uYOu1pWJmBeBGfZJbpqWAZnPjT4bf6QdCHcMrpazAKJt gH/REWzs5sm3Ladclr9r/caqbwt7AzWNnb+I9yq+DRlE84uuP6ekGFgsPAfrn0z4 VJdhc+BRn1zlTTPBEcfl60aCDVuuEvcmRaDh7jEaaYveJeHVpkCgm4H+lPH32/RE souSnmV5vjg8EE2xO29iMCD88/Qb/67hlHS+IsVdHIRqVDXE5dx+AlbGxcsllmR9 sUCreea5JtDLYygNzwoyxTNqzUteGbFHN4D+tEBJYoeBqQryw+Yk+ywBj2SfKBCg 2KlmADBU17xvB4+v6wkdEbC64VAp2rCiy1M9X9NEUw== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvfedrfeefhedgtdduucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhephffvvefujghffffkfgggtgesthdttddttdertdenucfhrhhomhepufgvrghn ucghhhhithhtohhnuceoshhpfihhihhtthhonhesshhpfihhihhtthhonhdrnhgrmhgvqe enucggtffrrghtthgvrhhnpedtffdvffeuleeuvdetkedvveehgfehvdegvefghfevudek geegleevgeejkeetkeenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrih hlfhhrohhmpehsphifhhhithhtohhnsehsphifhhhithhtohhnrdhnrghmvg X-ME-Proxy: Feedback-ID: i23c04076:Fastmail Original-Received: by mail.messagingengine.com (Postfix) with ESMTPA; Thu, 22 Sep 2022 17:38:34 -0400 (EDT) Original-Received: by melete.silentflame.com (Postfix, from userid 1000) id 5469D7F2C62; Thu, 22 Sep 2022 14:38:33 -0700 (MST) In-Reply-To: (Stefan Monnier's message of "Thu, 22 Sep 2022 14:45:19 -0400") Received-SPF: pass client-ip=66.111.4.28; envelope-from=spwhitton@spwhitton.name; helo=out4-smtp.messagingengine.com X-Spam_score_int: -27 X-Spam_score: -2.8 X-Spam_bar: -- X-Spam_report: (-2.8 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_LOW=-0.7, RCVD_IN_MSPIKE_H3=0.001, RCVD_IN_MSPIKE_WL=0.001, SPF_HELO_PASS=-0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Original-Sender: "Emacs-devel" Xref: news.gmane.io gmane.emacs.devel:296010 Archived-At: Hello Stefan, Thank you for reviewing. On Thu 22 Sep 2022 at 02:45PM -04, Stefan Monnier wrote: >> @@ -296,8 +305,27 @@ FILE-OR-LIST is the name of a working file; it may be a list of >> files or be nil (to execute commands that don't expect a file >> name or set of files). If an optional list of FLAGS is present, >> that is inserted into the command line before the filename. >> + >> +If `vc-want-edit-command-p' is non-nil, prompt the user to edit >> +COMMAND and FLAGS before execution. > > This prompting seems a bit too specific to particular callers of this > function, so I suggest you replace it with some kind of hook which takes > the command as arg and returns the command to use instead. It should be > a `-function` with a default value of `identity`. Each command that uses this would have to ensure that the prompting function is removed again once used, right? How would you suggest handling that? Perhaps we could have a macro doing something like what we see in vc-exec-after, `add-once-function' or something. >> @@ -327,6 +355,8 @@ case, and the process object in the asynchronous case." >> (string= (buffer-name) buffer)) >> (eq buffer (current-buffer))) >> (vc-setup-buffer buffer)) >> + (run-hook-with-args 'vc-pre-command-functions >> + command file-or-list flags) >> ;; If there's some previous async process still running, just kill it. >> (let ((squeezed (remq nil flags)) >> (inhibit-read-only t) > > Any chance this hook can be merged with the one I propose above? > If not, please clarify (in their doc) how&why they differ. Possibly vc-do-async-command and vc-git--pushpull can invoke the prompting function themselves, thereby obtaining the new command and arguments, and then add-function a constant function so that vc-do-command gets the new values. Is that something like what you have in mind? > Your commit message says: > > (vc-do-async-command): Use the new hook to insert into BUFFER the > command that's next to be run. > > but I don't understand what this changes does. > Wasn't it already inserted into BUFFER? What's changed? Previously it was inserted without going via the hook. >> (defun vc-git--pushpull (command prompt extra-args) >> "Run COMMAND (a string; either push or pull) on the current Git branch. >> If PROMPT is non-nil, prompt for the Git command to run." >> (let* ((root (vc-git-root default-directory)) >> (buffer (format "*vc-git : %s*" (expand-file-name root))) >> - (git-program vc-git-program) >> - args) >> - ;; If necessary, prompt for the exact command. >> - ;; TODO if pushing, prompt if no default push location - cf bzr. >> - (when prompt >> - (setq args (split-string >> - (read-shell-command >> - (format "Git %s command: " command) >> - (format "%s %s" git-program command) >> - 'vc-git-history) >> - " " t)) >> - (setq git-program (car args) >> - command (cadr args) >> - args (cddr args))) >> - (setq args (nconc args extra-args)) >> + ;; TODO if pushing, prompt if no default push location - cf bzr. >> + (vc-want-edit-command-p prompt)) >> (require 'vc-dispatcher) >> - (apply #'vc-do-async-command buffer root git-program command args) >> + (when vc-want-edit-command-p >> + (with-current-buffer (get-buffer-create buffer) >> + (add-hook 'vc-pre-command-functions >> + (pcase-lambda (_ _ `(,new-command . ,new-args)) >> + (setq command new-command extra-args new-args)) >> + nil t))) >> + (apply #'vc-do-async-command >> + buffer root vc-git-program command extra-args) >> (with-current-buffer buffer >> (vc-run-delayed >> (vc-compilation-mode 'git) >> (setq-local compile-command >> - (concat git-program " " command " " >> - (mapconcat #'identity args " "))) >> + (concat vc-git-program " " command " " >> + (mapconcat #'identity extra-args " "))) >> (setq-local compilation-directory root) >> ;; Either set `compilation-buffer-name-function' locally to nil >> ;; or use `compilation-arguments' to set `name-function'. > > IIUC the (git-program vc-git-program) binding allowed that var to have > different values in different buffers. This change instead presumes > `vc-git-program` is the same in all buffers. Not sure it matters, but > we've been bitten by similar things in VC. It's only the use of vc-git-program in compile-command that now works differently, right? The value passed to vc-do-async-command should still be the same buffer-local value if there is one. -- Sean Whitton