From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Dmitry Gutov Newsgroups: gmane.emacs.bugs Subject: bug#52349: 29.0.50; vc-git and diff-mode: stage hunks Date: Sun, 13 Feb 2022 03:32:19 +0200 Message-ID: References: <268cee0d-465d-b862-d1d6-f5da4d69e737@inventati.org> <9106387a-98cf-396f-bf45-ccb04581787b@yandex.ru> <864k7kfd56.fsf@mail.linkov.net> <86fsr3uen2.fsf@mail.linkov.net> <95d2d999-49ad-13c4-9f25-0935650a1e42@yandex.ru> <8635ktjfll.fsf@mail.linkov.net> <36b963e4-5af7-dd9e-af92-4404541b9f4d@yandex.ru> <86v8xjx45h.fsf@mail.linkov.net> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="3565"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.5.0 Cc: 52349@debbugs.gnu.org To: Juri Linkov Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Sun Feb 13 02:33:19 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 1nJ3lO-0000ly-LI for geb-bug-gnu-emacs@m.gmane-mx.org; Sun, 13 Feb 2022 02:33:18 +0100 Original-Received: from localhost ([::1]:33056 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1nJ3lN-0002Iu-An for geb-bug-gnu-emacs@m.gmane-mx.org; Sat, 12 Feb 2022 20:33:17 -0500 Original-Received: from eggs.gnu.org ([209.51.188.92]:59254) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1nJ3l8-0002Im-83 for bug-gnu-emacs@gnu.org; Sat, 12 Feb 2022 20:33:02 -0500 Original-Received: from debbugs.gnu.org ([209.51.188.43]:42136) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1nJ3l7-0002em-Uj for bug-gnu-emacs@gnu.org; Sat, 12 Feb 2022 20:33:01 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1nJ3l7-0007JL-SK for bug-gnu-emacs@gnu.org; Sat, 12 Feb 2022 20:33:01 -0500 X-Loop: help-debbugs@gnu.org Resent-From: Dmitry Gutov Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Sun, 13 Feb 2022 01:33:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 52349 X-GNU-PR-Package: emacs Original-Received: via spool by 52349-submit@debbugs.gnu.org id=B52349.164471595028066 (code B ref 52349); Sun, 13 Feb 2022 01:33:01 +0000 Original-Received: (at 52349) by debbugs.gnu.org; 13 Feb 2022 01:32:30 +0000 Original-Received: from localhost ([127.0.0.1]:36033 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1nJ3kb-0007Ic-K1 for submit@debbugs.gnu.org; Sat, 12 Feb 2022 20:32:29 -0500 Original-Received: from mail-wr1-f43.google.com ([209.85.221.43]:46056) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1nJ3ka-0007IP-4b for 52349@debbugs.gnu.org; Sat, 12 Feb 2022 20:32:28 -0500 Original-Received: by mail-wr1-f43.google.com with SMTP id p9so706175wra.12 for <52349@debbugs.gnu.org>; Sat, 12 Feb 2022 17:32:28 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=sender:message-id:date:mime-version:user-agent:subject :content-language:to:cc:references:from:in-reply-to :content-transfer-encoding; bh=Z4+rUrGUQzquC17QyHw93bVEagP1mz1eN6P81f7aobY=; b=YweW52HT9z9HrZpqTaaE3OIfeA88GC7uovZSi5pohFyVqzEeoDUCExTRF81GoTco2b yFIiU1f6W2O3cv8/S1MU/flCu4bq7KX9fz3849bqXn9zd7Hpe4MCJkX6Ujn7+6pFIMbB 4N83nKqOaBdTm2vqXr5P0vbPTntgK69fVEiBIpPb1Ubvf2gbmZ07ZysyD9eFB7el6hI0 +EABB/ASqMX3QBaU+l1jKzlqtfYRnCILI7Ljr8fpFueW1WYTIxnuYmOhFIMuKzwkw2MX v+V2ZdtRkXSnCFZZlUsskAY2xS2liS237cmRO9hWMxr9WSTzq/4qVwnVA4iHzWpKoBDo PApg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:sender:message-id:date:mime-version:user-agent :subject:content-language:to:cc:references:from:in-reply-to :content-transfer-encoding; bh=Z4+rUrGUQzquC17QyHw93bVEagP1mz1eN6P81f7aobY=; b=WEqk89Xx+zKWdYuKy2kKxSKM3eRYbmDRWPaNALR1aPkxzXUwgxexzMrBI+lblLy5t4 7awl65gbTTXtWB91XpNCVuC/EKoBhiWCKeigrSbB2qok+G0hBL2za/QFzjW+/mLMGJF9 jUgvyebBBH23MgWnWWttWVkvUgqE01eyshf5I3f80uCeeGfG/Zr7B/swG8PG204jwUFX BtZyJkDFGBHOz6glIdsHEulxgchDjI6nlMS1iLgtoj4tX/E6Yj3yI1Wt7Pl3+qadL1Zp 7/hldx+KyRLsUX1KPLVSw9e+K76oEZskJ95UCQwugn1TrPDwrb/szSADw+Drj+7bzcrS qwvg== X-Gm-Message-State: AOAM532OFV0m8/rUaGFOSfMgh1+Ejznv6/14i2QSeDUw9K91JN00VlRt ooTc7qy14my2C+XXvydi9c0= X-Google-Smtp-Source: ABdhPJzf5uIF93GUwHMBWTyK1axEXhKppQyHpisoEHutwza298pzpyqYLLp8DTS6BOubG8TWeF5hOQ== X-Received: by 2002:a5d:5043:: with SMTP id h3mr6150253wrt.394.1644715941806; Sat, 12 Feb 2022 17:32:21 -0800 (PST) Original-Received: from [192.168.0.6] ([46.251.119.176]) by smtp.googlemail.com with ESMTPSA id az13sm4129253wrb.39.2022.02.12.17.32.20 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Sat, 12 Feb 2022 17:32:21 -0800 (PST) Content-Language: en-US In-Reply-To: <86v8xjx45h.fsf@mail.linkov.net> 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:226758 Archived-At: On 12.02.2022 21:42, Juri Linkov wrote: >> But could you explain the case when the changes to 'vc-diff-internal' are >> going to be used? If those are only for log-edit-show-diff, I think it'd be >> better if the new logic was implemented in the new value of >> log-edit-diff-function, rather than having it spliced into the common code >> path. Would that result in a lot of code duplication? > > Not much code duplication, thanks for the suggestion, the patch below > sets log-edit-diff-function for log-edit-show-diff. Nice. >> It might also be worth it to thread the 'patch-buffer' value through the >> backend method arguments (the actual value will be the patch string), so >> that vc-git-checkin gets it in the 4th argument, rather than having it call >> (derived-mode-p 'diff-mode) (this feels a little brittle: I suppose which >> buffer is current during this call might change in the future). It would >> also automatically weed out backends which don't support this feature, >> rather than having an attempt to commit from a diff buffer using Hg fail >> silently. > > I agree that in the first version ‘(derived-mode-p 'diff-mode)’ was brittle. > But changing the established API with a new argument doesn't look right. We could make sure to call the function with the current number of arguments when a patch-buffer is not used, and with the additional one when it is used. Which would automatically force an abort when a backend does not support this feature. > So the next version below uses the buffer-local variable ‘vc-patch-string’. > In other backends such as Hg it shouldn't fail silently, but it will be > just less granular, and will commit whole files instead of edited diffs. That's not the worst approach, but it's bound to trip up some users who get used to the new feature's behavior with Git repos, and then try it with a different one that does not support this feature properly (such as Hg). Without any warning, they will see a different behavior. What do people think? Is that not a problem? > There is a new problem however. After starting vc-log-edit from *vc-diff*, > and using log-edit-show-diff, it reuses the original buffer *vc-diff*. > This is not a problem, because the buffer-local variable ‘vc-patch-string’ > is saved in the *vc-log* buffer. But after deleting *vc-diff*, log-edit-done > fails on the deleted vc-parent-buffer with > > Debugger entered--Lisp error: (error "Selecting deleted buffer") > vc-finish-logentry() > funcall-interactively(vc-finish-logentry) > log-edit-done() > > But this is an old problem. The same error is signaled > after typing ‘v’ in *vc-dir* buffer, then deleting the > original *vc-dir* buffer, and trying to type ‘C-c C-c’ > (log-edit-done) in the *vc-log* buffer. Seems like a slightly different issue, though. Speaking of myself only, I can see myself casually killing the vc-diff buffer (especially if I just displayed it with C-c C-d), but it seems less likely that I would kill the vc-dir buffer when completing the commit. I suppose 'vc-diff-patch' could use a different buffer name than "*vc-diff*" and thus avoid reusing that buffer? If it brings other problems somehow, oh well. An old bug is something we can live with. But also note that if the patch string was passed as an argument to the backend action, this problem might be avoided as well.