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#55871: Acknowledgement (27.1; vc-git.el log view 'a', 'f', 'd' do not work when following renames) Date: Thu, 18 Aug 2022 05:10:16 +0300 Message-ID: <78f97339-2aca-0dbd-4cb4-3532af78a895@yandex.ru> References: 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="35055"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.9.1 To: =?UTF-8?Q?Nicol=C3=A1s?= Ojeda =?UTF-8?Q?B=C3=A4r?= , 55871@debbugs.gnu.org Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Thu Aug 18 04:11:31 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 1oOV0J-0008qh-EV for geb-bug-gnu-emacs@m.gmane-mx.org; Thu, 18 Aug 2022 04:11:27 +0200 Original-Received: from localhost ([::1]:44958 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1oOV0H-0008PJ-Qo for geb-bug-gnu-emacs@m.gmane-mx.org; Wed, 17 Aug 2022 22:11:25 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:34902) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1oOUzy-0008P3-QO for bug-gnu-emacs@gnu.org; Wed, 17 Aug 2022 22:11:06 -0400 Original-Received: from debbugs.gnu.org ([209.51.188.43]:35353) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1oOUzu-0000Tk-J0 for bug-gnu-emacs@gnu.org; Wed, 17 Aug 2022 22:11:06 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1oOUzu-0004oW-EM for bug-gnu-emacs@gnu.org; Wed, 17 Aug 2022 22:11:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Dmitry Gutov Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Thu, 18 Aug 2022 02:11:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 55871 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch Original-Received: via spool by 55871-submit@debbugs.gnu.org id=B55871.166078862918462 (code B ref 55871); Thu, 18 Aug 2022 02:11:02 +0000 Original-Received: (at 55871) by debbugs.gnu.org; 18 Aug 2022 02:10:29 +0000 Original-Received: from localhost ([127.0.0.1]:53334 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1oOUzM-0004nh-Fj for submit@debbugs.gnu.org; Wed, 17 Aug 2022 22:10:28 -0400 Original-Received: from mail-wr1-f43.google.com ([209.85.221.43]:36491) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1oOUzJ-0004nH-20 for 55871@debbugs.gnu.org; Wed, 17 Aug 2022 22:10:26 -0400 Original-Received: by mail-wr1-f43.google.com with SMTP id j7so152207wrh.3 for <55871@debbugs.gnu.org>; Wed, 17 Aug 2022 19:10:25 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:in-reply-to:from:references:to :content-language:subject:user-agent:mime-version:date:message-id :sender:from:to:cc; bh=Tt+J7JYldQbuykSOurmf4m6J3AjUtNdzPufCitLRHM0=; b=ipv5u9+5kPRN2dnOlfTJ0U/kW7K2J8ge9c1p7wMGTUSdY1TP3P9ymZ4SRSddtvPZlQ WsfA488nAE72/KKA8M9EOCDubM8XtekWaImkMH2peHTIq8zqDRqDnIPG7q8Zk+WOMMW2 8aKDnn0b4C4CORCDj4lBAGBpgmuQaIe4G21QWHASD6KNsWP7Btikc+dPfF6bO4jycZ3T iduFQCQCHDQMP6eyTRpZvfEFDzmevJX0zIjkYzN7DiikjBJJEHr4IQnbcn+WOQId3d7s vWhAKxRC/JN2qK+hVAgQdvnCiw7KZSBm40Mbl2XyTS7eDQABtOhfFn3WKE43wUxe8j7k 84tw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:in-reply-to:from:references:to :content-language:subject:user-agent:mime-version:date:message-id :sender:x-gm-message-state:from:to:cc; bh=Tt+J7JYldQbuykSOurmf4m6J3AjUtNdzPufCitLRHM0=; b=bfSTHGh87uBTcEUQpmDBui/lSZih9hpUSsRZapR3AeMx2I4tYoziluk4cahGQm+KK6 xtkxgp4YErSQbe4ZCjNXb3hNTOBlRSyF4ttN1jM1tWqp6g7JNfmkbw3R4l0f/YMh+rWN JGkV0ZW6kSfTtN3yv7+QlyCyM1qGqFDKnawKz5CuTxA83r78xXSUozjL16y1Rl9uP+0Q gfl4aRKPJao1Yix1NzwqF+YIpXQv+AFQvOzCDk2aJwsu7dOoII8lT0GFP/F9dXqTEIdh 0EfPf6FuvzC98Xr5lXjrKkEun1gVFgUU7IS/ueI0wCWL+qEWsWjgd3twf//o07HM3pAU RHIQ== X-Gm-Message-State: ACgBeo0siDDOdBmmqaoLadRBLF+hu0pj4oix/ILH5wcDiZOI+8R0L0N4 EazAMFNdrSbcVyvPGAH7Kbw= X-Google-Smtp-Source: AA6agR6G4TlXQ8nHXZeakJCV14BuOK8jW2VVfm6aFJttVakXqP+eNQ6KukjBtt+ML2UrkWiqHhV6/g== X-Received: by 2002:a5d:4d41:0:b0:21e:a5e3:cb9b with SMTP id a1-20020a5d4d41000000b0021ea5e3cb9bmr287801wru.523.1660788618854; Wed, 17 Aug 2022 19:10:18 -0700 (PDT) Original-Received: from [192.168.0.6] ([46.251.119.176]) by smtp.googlemail.com with ESMTPSA id g11-20020a5d488b000000b0022377df817fsm57931wrq.58.2022.08.17.19.10.17 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 17 Aug 2022 19:10:18 -0700 (PDT) Content-Language: en-US In-Reply-To: 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:240120 Archived-At: Hi Nicholas, On 10.06.2022 20:31, Nicolás Ojeda Bär wrote: > Dear all, > > Attached is a patch that solves some of the problems in this issue (not all). > > Its main feature is that it should not modify anything if you are not > using `vc-git-print-log-follow`. > > If you are, the patch makes it possible to use `a`, `f` and `d` from > inside the `*vc-change-log*` buffer. > > The patch itself is very much based on the one proposed in > https://debbugs.gnu.org/cgi/bugreport.cgi?bug=8756#53, with some > changes to make sure that nothing changes if `vc-git-print-log-follow` > is not being used. Additionally, a horrible HACK (see commit message) > is used to allow `vc-git.el` to discriminate between `d` and `D` in > the `*vc-change-log*` buffer, as these two functions require different > Git commands to be executed when following renames (this is also > discussed in the just linked bug report). Thanks for your help with updating this patch. I've tried it out, and it seems functional, which is great. First of all, how do you feel about signing copyright assignment for Emacs? AFAICT you haven't done so yet, and this patch borders on our allowed limit without assignment (if I take only the lines that changed substantially from Dmitry Dzhus's version). It would probably be easier to get the assignment going anyway, so you continue to be able to contribute. There are also a few of improvements I'm eyeing here (not requirements for acceptance -- I can do them myself, if you like): 1) vc-git--shalist-raw doesn't seem to serve much of a purpose. It is there only to delay the conversion from the raw string into a list? 2) It would be better to produce shalist entirely lazily. I'm not sure what's stopping us from doing that, given that vc-git--shalist knows how to find the log buffer. Finding the values for all the arguments might be a problem, so we can save the args (start-revision, limit) to a new var. 3) git-log-view-diff-whole-changeset might be unnecessary. vc-git-diff could look up two file names, for both revisions (the current and the one corresponding to the previous revision in the sha list), or log-view-diff-common could pass explicitly the previous revision from the already-printed list, rather than fetch it using (vc-call-backend log-view-vc-backend 'previous-revision nil fr) -- I'm not sure why it's doing it this way. At first I just changed vc-git-diff to do this: (let ((file1 (vc-git--rev-to-filename rev1)) (file2 (vc-git--rev-to-filename rev2))) (when (or file1 file2) ;; Run diff from the repository root because our file names are ;; relative to it (setq default-directory (vc-git-root default-directory)) (when file1 (push file1 files)) (when file2 (push file2 files)))) ...and reverted the change to vc-git-previous-revision, and that almost worked, except a revision that did rename only showed one side (addition, not deletion). WDYT? I experimented with --follow myself in the past, and it is annoying in that it skips commits, some of which are visible in the log when you don't use --follow, details here: https://stackoverflow.com/questions/46487476/git-log-follow-graph-skips-commits So I figured the approach in (3) has something to do with it. But it seems not to be the case.