From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Eli Zaretskii Newsgroups: gmane.emacs.bugs Subject: bug#57400: 29.0.50; Support sending patches from VC directly Date: Tue, 04 Oct 2022 09:41:50 +0300 Message-ID: <83r0zow0nl.fsf@gnu.org> References: <84v8qgn1z9.fsf@iki.fi> <87h71zo3p8.fsf@posteo.net> <87sfljmgwz.fsf@posteo.net> <87y1twvima.fsf@posteo.net> <878rlw681a.fsf@gnus.org> <87r0zovbzr.fsf@posteo.net> <87mtacvag8.fsf@posteo.net> Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="38775"; mail-complaints-to="usenet@ciao.gmane.io" Cc: larsi@gnus.org, 57400@debbugs.gnu.org, ane@iki.fi To: Philip Kaludercic Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Tue Oct 04 08:44:09 2022 Return-path: Envelope-to: geb-bug-gnu-emacs@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 1ofbey-0009rx-0T for geb-bug-gnu-emacs@m.gmane-mx.org; Tue, 04 Oct 2022 08:44:08 +0200 Original-Received: from localhost ([::1]:58544 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1ofbew-0004kh-MD for geb-bug-gnu-emacs@m.gmane-mx.org; Tue, 04 Oct 2022 02:44:06 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:39116) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1ofbdu-0004jQ-DP for bug-gnu-emacs@gnu.org; Tue, 04 Oct 2022 02:43:02 -0400 Original-Received: from debbugs.gnu.org ([209.51.188.43]:53098) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1ofbdu-0007kZ-5t for bug-gnu-emacs@gnu.org; Tue, 04 Oct 2022 02:43:02 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1ofbdu-0003ZC-1G for bug-gnu-emacs@gnu.org; Tue, 04 Oct 2022 02:43:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Eli Zaretskii Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Tue, 04 Oct 2022 06:43:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 57400 X-GNU-PR-Package: emacs Original-Received: via spool by 57400-submit@debbugs.gnu.org id=B57400.166486572713636 (code B ref 57400); Tue, 04 Oct 2022 06:43:01 +0000 Original-Received: (at 57400) by debbugs.gnu.org; 4 Oct 2022 06:42:07 +0000 Original-Received: from localhost ([127.0.0.1]:52176 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1ofbd0-0003Xs-Fg for submit@debbugs.gnu.org; Tue, 04 Oct 2022 02:42:06 -0400 Original-Received: from eggs.gnu.org ([209.51.188.92]:50910) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1ofbcy-0003XM-Oq for 57400@debbugs.gnu.org; Tue, 04 Oct 2022 02:42:05 -0400 Original-Received: from fencepost.gnu.org ([2001:470:142:3::e]:52416) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1ofbcr-0007dM-D6; Tue, 04 Oct 2022 02:41:57 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=gnu.org; s=fencepost-gnu-org; h=References:Subject:In-Reply-To:To:From:Date: mime-version; bh=spPi2l0HkmIT2PS064HEp2QQnoLtLq/4ypdeHs8uWkk=; b=fYXgMRmJlJ2r bAFbCjDyMXHjXA2TsdOtIOR1Qld9eL+W0RGz6ILx2dDpd2qzbG1kViJ1Gv2J96UUVjk3BCh5nd11q XXlcxdM6x6b+QjwkTrGsqKqZCjOwyoLgGKxpzeRRfyOXb7ks8yo08dQwcpPmzZWVEpH1F9jE23GEq jwmF6I1TfN7qJGxFf8MCYPyj9dvJKFEqSRDcDCVxNRSm/LneD+2eb2L+nh9+vQ9AzImC32BzkQfNI ypceJ4FXH02w6qfmaoFnczRfLTa7oT0DDhGcLyb5e+1yAQhq33hDwrZLRnE3f/Fb8TDH8Ptxut34v f7IXtHQ503pXt0lpaZFu2w==; Original-Received: from [87.69.77.57] (port=2191 helo=home-c4e4a596f7) by fencepost.gnu.org with esmtpsa (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1ofbcp-0006DA-Ru; Tue, 04 Oct 2022 02:41:56 -0400 In-Reply-To: <87mtacvag8.fsf@posteo.net> (message from Philip Kaludercic on Mon, 03 Oct 2022 21:55:35 +0000) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Original-Sender: "bug-gnu-emacs" Xref: news.gmane.io gmane.emacs.bugs:244363 Archived-At: > Cc: 57400@debbugs.gnu.org, Antoine Kalmbach > From: Philip Kaludercic > Date: Mon, 03 Oct 2022 21:55:35 +0000 > > >> Others should indeed run vc-diff, I > >> think. > > > > That seem possible, but for that to work I will need a generic way to > > detect the predecessor of a commit and extract a commit message. > > Currently, I am not sure if this can be done. > > I had forgotten about `previous-revision', so that was easy and I > decided to fall back to a generic subject as that can be easily revised: Thanks. A few comments below. > +(defun vc-git-prepare-patch (rev) > + (with-temp-buffer > + (call-process vc-git-program nil t nil "format-patch" > + "--no-numbered" "--stdout" > + ;; From gitrevisions(7): ^ means the th parent > + ;; (i.e. ^ is equivalent to ^1). As a > + ;; special rule, ^0 means the commit itself and > + ;; is used when is the object name of a tag > + ;; object that refers to a commit object. > + (concat rev "^0")) This needs to set up decoding of the stuff Git outputs, because it is usually in UTF-8 (but could be in some other encoding), and the defaults could be different, depending on the locale. See vc-git-log-output-coding-system and its use elsewhere. Or maybe you should just use vc-git--call here, which handles that already, and uses process-file instead of call-process, so this will work via Tramp: an additional benefit. > + (let (filename subject body) > + ;; Save the patch in a temporary file if required > + (when (bound-and-true-p vc-compose-patches-inline) > + (setq filename (make-temp-file "vc-git-prepare-patch")) > + (write-region nil nil filename)) ;FIXME: Clean up By "clean up" did you mean delete the temporary file? > + ;; Return the extracted data > + (list :filename filename > + :subject subject > + :body body)))) I think this should include :encoding ENCODING, to provide the temporary file's encoding to the caller. The encoding should be the same as buffer-file-coding-system in the buffer which you write above. > -(defun vc-read-revision (prompt &optional files backend default initial-input) > +(defun vc-read-revision (prompt &optional files backend default initial-input multiple) This API change warrants a NEWS entry, I think. > (cond > ((null files) > (let ((vc-fileset (vc-deduce-fileset t))) ;FIXME: why t? --Stef > @@ -1920,9 +1928,16 @@ vc-read-revision > (let ((completion-table > (vc-call-backend backend 'revision-completion-table files))) > (if completion-table > - (completing-read prompt completion-table > - nil nil initial-input 'vc-revision-history default) > - (read-string prompt initial-input nil default)))) > + (funcall > + (if multiple #'completing-read-multiple #'completing-read) > + prompt completion-table nil nil initial-input 'vc-revision-history default) > + (let ((answer (read-string prompt initial-input nil default))) > + (if multiple > + (split-string answer "[ \t]*,[ \t]*") > + answer))))) > + > +(defun vc-read-multiple-revisions (prompt &optional files backend default initial-input) > + (vc-read-revision prompt files backend default initial-input t)) > > (defun vc-diff-build-argument-list-internal (&optional fileset) > "Build argument list for calling internal diff functions." > @@ -3243,6 +3258,73 @@ vc-update-change-log > (vc-call-backend (vc-responsible-backend default-directory) > 'update-changelog args)) > > +(defcustom vc-compose-patches-inline nil > + "Non-nil means that `vc-compose-patch' creates a single message." This is rather cryptic, IMO, and will benefit from more detailed description, after the initial line. Also, I suggest a slight rewording of the above sentence: If non-nil, `vc-compose-patch' will create a single email message. > + :type 'boolean > + :safe #'booleanp) The :version tag is missing. > +(declare-function message-goto-body "message" (&optional interactive)) > +(declare-function message--name-table "message" (orig-string)) Can we not force the use of message.el? Can we use mail-user-agent instead, and use its properties to find out the compose function the user has set up, like "C-x m" does? This would probably eliminate quite a few problems with user email setup, which might not support message.el. > +(defun vc-compose-patch (addressee subject revisions) > + "Compose a message sending REVISIONS to ADDRESSEE with SUBJECT." "Compose an email message to send REVISIONS to ADDRESSEE with SUBJECT." The "email" part is important, because we cannot assume the reader of the doc string is necessarily aware of the context. > + (interactive (save-current-buffer > + (require 'message) > + (vc-ensure-vc-buffer) > + (let ((revs (vc-read-multiple-revisions "Revisions: ")) to) > + (while (null (setq to (completing-read-multiple > + "Whom to send: " Instead "Whom to send:" I suggest just "Addressee:" or maybe even "Send patches to:" > + (compose-mail addressee > + (or (plist-get patch :subject) > + (concat > + "Patch for " ;guess > + (file-name-nondirectory > + (directory-file-name > + (vc-root-dir))))) > + nil nil nil nil > + `((exit-recursive-edit))) > + (message-goto-body) compose-mail doesn't necessarily invoke message.el functions, so message-goto-body is not necessarily appropriate here.