* bug#55871: 27.1; vc-git.el log view 'a', 'f', 'd' do not work when following renames @ 2022-06-09 9:54 Nicolás Ojeda Bär [not found] ` <handler.55871.B.16547851264967.ack@debbugs.gnu.org> 0 siblings, 1 reply; 18+ messages in thread From: Nicolás Ojeda Bär @ 2022-06-09 9:54 UTC (permalink / raw) To: 55871 When pressing 'C-x v l' in a git versioned file that has been renamed, it is possible to get a full history (ie including renames) by setting 'vc-git-print-log-follow' to t. However, if doing this, the functions - 'd' (log-view-diff) - 'f' (log-view-find-revision) - 'a' (log-view-annotate-revision) stop working with revisions "before" the rename. There is a previous discussion of this problem in https://debbugs.gnu.org/cgi/bugreport.cgi?bug=8756 but the discussion ends by suggesting to open a new bug report about this issue, which I am doing here. In GNU Emacs 27.1 (build 1, x86_64-w64-mingw32) of 2020-08-21 built on CIRROCUMULUS Repository revision: 86d8d76aa36037184db0b2897c434cdaab1a9ae8 Repository branch: HEAD Windowing system distributor 'Microsoft Corp.', version 10.0.22000 System Description: Microsoft Windows 10 Pro (v10.0.2009.22000.708) Recent messages: process Mark saved where search started Saving file c:/cygwin64/home/nojebar/mlfi/applications/scheduler/main.mf... Mark set Wrote c:/cygwin64/home/nojebar/mlfi/applications/scheduler/main.mf Wrote c:/cygwin64/home/nojebar/mlfi/applications/scheduler/main.mf (3 pending errors, use C-c C-x to jump) Mark saved where search started [3 times] Mark set Mark saved where search started [13 times] Quit [2 times] Quit Configured using: 'configure --without-dbus --host=x86_64-w64-mingw32 --without-compress-install 'CFLAGS=-O2 -static'' Configured features: XPM JPEG TIFF GIF PNG RSVG SOUND NOTIFY W32NOTIFY ACL GNUTLS LIBXML2 HARFBUZZ ZLIB TOOLKIT_SCROLL_BARS MODULES THREADS JSON PDUMPER LCMS2 GMP Important settings: value of $LANG: fr_FR.UTF-8 locale-coding-system: cp1252 Major mode: caml Minor modes in effect: shell-dirtrack-mode: t merlin-mode: t save-place-mode: t winner-mode: t savehist-mode: t show-paren-mode: t delete-selection-mode: t display-time-mode: t global-auto-revert-mode: t tooltip-mode: t global-eldoc-mode: t electric-indent-mode: t mouse-wheel-mode: t file-name-shadow-mode: t global-font-lock-mode: t font-lock-mode: t blink-cursor-mode: t auto-composition-mode: t auto-encryption-mode: t auto-compression-mode: t size-indication-mode: t column-number-mode: t line-number-mode: t transient-mark-mode: t Load-path shadows: c:/cygwin64/home/nojebar/mlfi/mlfi-ins/share/emacs/site-lisp/ocp-indent hides ~/scripts/install/emacs/ocp-indent c:/cygwin64/home/nojebar/.emacs.d/elpa/jsonrpc-1.0.15/jsonrpc hides c:/Program Files/emacs-27.1/share/emacs/27.1/lisp/jsonrpc c:/cygwin64/home/nojebar/.emacs.d/elpa/xref-1.4.1/xref hides c:/Program Files/emacs-27.1/share/emacs/27.1/lisp/progmodes/xref c:/cygwin64/home/nojebar/.emacs.d/elpa/project-0.8.1/project hides c:/Program Files/emacs-27.1/share/emacs/27.1/lisp/progmodes/project c:/cygwin64/home/nojebar/.emacs.d/elpa/flymake-1.2.2/flymake hides c:/Program Files/emacs-27.1/share/emacs/27.1/lisp/progmodes/flymake c:/cygwin64/home/nojebar/.emacs.d/elpa/seq-2.23/seq hides c:/Program Files/emacs-27.1/share/emacs/27.1/lisp/emacs-lisp/seq c:/cygwin64/home/nojebar/.emacs.d/elpa/eldoc-1.12.0/eldoc hides c:/Program Files/emacs-27.1/share/emacs/27.1/lisp/emacs-lisp/eldoc ~/scripts/install/emacs/cl-lib hides c:/Program Files/emacs-27.1/share/emacs/27.1/lisp/emacs-lisp/cl-lib Features: (shadow sort mail-extr emacsbug message rmc puny format-spec rfc822 mml mml-sec epa epg epg-config gnus-util rmail rmail-loaddefs text-property-search time-date mm-decode mm-bodies mm-encode mail-parse rfc2231 mailabbrev gmm-utils mailheader sendmail rfc2047 rfc2045 ietf-drums mm-util mail-prsvr mail-utils add-log log-view pcvs-util help-fns radix-tree vc-annotate ffap grep thingatpt cl-extra help-mode shell pcomplete smerge-mode diff vc vc-dispatcher misearch multi-isearch merlin-xref xref project merlin-cap merlin pcase caml-types crm caml advice rx compile derived imenu cygwin-mount ange-ftp comint ansi-color ocamlformat ocp-indent caml-font vc-git diff-mode easy-mmode dired-x dired dired-loaddefs saveplace edmacro kmacro winner ring savehist paren delsel time autorevert filenotify info proof-site proof-autoloads package easymenu browse-url url-handlers url-parse auth-source cl-seq eieio eieio-core cl-macs eieio-loaddefs password-cache json subr-x map url-vars seq byte-opt gv bytecomp byte-compile cconv cl-loaddefs cl-lib tooltip eldoc electric uniquify ediff-hook vc-hooks lisp-float-type mwheel dos-w32 ls-lisp disp-table term/w32-win w32-win w32-vars term/common-win tool-bar dnd fontset image regexp-opt fringe tabulated-list replace newcomment text-mode elisp-mode lisp-mode prog-mode register page tab-bar menu-bar rfn-eshadow isearch timer select scroll-bar mouse jit-lock font-lock syntax facemenu font-core term/tty-colors frame minibuffer cl-generic cham georgian utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao korean japanese eucjp-ms cp51932 hebrew greek romanian slovak czech european ethiopic indian cyrillic chinese composite charscript charprop case-table epa-hook jka-cmpr-hook help simple abbrev obarray cl-preloaded nadvice loaddefs button faces cus-face macroexp files text-properties overlay sha1 md5 base64 format env code-pages mule custom widget hashtable-print-readable backquote threads w32notify w32 lcms2 multi-tty make-network-process emacs) Memory information: ((conses 16 298590 19356) (symbols 48 12729 1) (strings 32 40782 1936) (string-bytes 1 1406019) (vectors 16 22528) (vector-slots 8 276819 12620) (floats 8 104 554) (intervals 56 42748 0) (buffers 1000 42)) ^ permalink raw reply [flat|nested] 18+ messages in thread
[parent not found: <handler.55871.B.16547851264967.ack@debbugs.gnu.org>]
* bug#55871: Acknowledgement (27.1; vc-git.el log view 'a', 'f', 'd' do not work when following renames) [not found] ` <handler.55871.B.16547851264967.ack@debbugs.gnu.org> @ 2022-06-10 17:31 ` Nicolás Ojeda Bär 2022-08-18 2:10 ` Dmitry Gutov 0 siblings, 1 reply; 18+ messages in thread From: Nicolás Ojeda Bär @ 2022-06-10 17:31 UTC (permalink / raw) To: 55871 [-- Attachment #1: Type: text/plain, Size: 829 bytes --] 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). Cheers, Nicolas [-- Attachment #2: 0001-vc-git.el-better-support-for-follow-mode.patch --] [-- Type: application/octet-stream, Size: 8087 bytes --] From d52f16607fcca4f80f1720b863ab920d15d9c0bb Mon Sep 17 00:00:00 2001 From: nojebar <nicolas.ojeda.bar@lexifi.com> Date: Fri, 10 Jun 2022 16:16:26 +0200 Subject: [PATCH] vc-git.el: better support for "follow" mode * lisp/vc/vc-git.el (vc-print-log): generate map between SHA-1's and filenames when printing the log of a single file in "follow" mode. (vc-git-find-revision): use mapping to find the correct filename to checkout when pressing 'f' in '*vc-change-log*'. (vc-git-diff): use mapping to find the correct filenames to diff when computing diffs between two versions of a file from the '*vc-change-log*' buffer. (vc-git-annotate-command): use mapping to pass the right filename when pressing 'a' in '*vc-change-log*'. (vc-git-previous-revision): use mapping to find previous revision of a file that has been renamed. Additionally a horrible hack (see next point) is done so that this function can differentiate between 'd' and 'D' (whole changeset diffing) since these two operations require different Git commands to be executed. This makes it possible to use the 'd' command in '*vc-change-log*' after renaming a file. * lisp/vc/log-view.el (log-view-diff-common): set a buffer-local variable 'git-log-view-diff-whole-changeset' to let vc-git.el differentiate between 'd' and 'D'. --- lisp/vc/log-view.el | 1 + lisp/vc/vc-git.el | 101 ++++++++++++++++++++++++++++++++++++++------ 2 files changed, 89 insertions(+), 13 deletions(-) diff --git a/lisp/vc/log-view.el b/lisp/vc/log-view.el index 415b1564ed..9685883504 100644 --- a/lisp/vc/log-view.el +++ b/lisp/vc/log-view.el @@ -601,6 +601,7 @@ log-view-diff-changeset (log-view-diff-common beg end t))) (defun log-view-diff-common (beg end &optional whole-changeset) + (setq-local git-log-view-diff-whole-changeset whole-changeset) (let* ((to (log-view-current-tag beg)) (fr-entry (log-view-current-entry end)) (fr (cadr fr-entry))) diff --git a/lisp/vc/vc-git.el b/lisp/vc/vc-git.el index 8937454d11..8634785a94 100644 --- a/lisp/vc/vc-git.el +++ b/lisp/vc/vc-git.el @@ -969,18 +969,77 @@ vc-git-checkin (if only (list "--only" "--") '("-a"))))) (if (and msg-file (file-exists-p msg-file)) (delete-file msg-file)))) +;;; '--follow' HANDLING + +(defvar vc-git--shalist-raw nil) +(defvar vc-git--shalist nil) + +(defun vc-git--make-shalist (buffer files start-revision limit) + "Store newline-separated list of revision hashes and file names +in vc-git--shalist-raw buffer-local variable." + (setq-local vc-git--shalist-raw nil) + (setq-local vc-git--shalist nil) + (with-temp-buffer + (set-process-filter + (apply #'vc-git-command nil + 'async files + (append + '("log" + "--follow" + "--name-only" + "--pretty=tformat:%H" + "--no-color") + ;; Tail revision must now its parent + (when limit (list "-n" (format "%s" (1+ limit)))) + (when start-revision (list start-revision)) + '("--"))) + (lambda (_p s) + (with-current-buffer buffer + (setq-local + vc-git--shalist-raw + (replace-regexp-in-string + "\n\n" "\n" + (concat vc-git--shalist-raw s)))))))) + +(defun vc-git--shalist () + "Return alternating list of SHA1 hashes and file names. +The list contains commit hashes and historical names for a file +in the current change log buffer." + (let ((vc-change-log (get-buffer "*vc-change-log*"))) + (when vc-change-log + (with-current-buffer vc-change-log + (cond + (vc-git--shalist vc-git--shalist) + (vc-git--shalist-raw + (setq-local vc-git--shalist + (split-string vc-git--shalist-raw "\n")))))))) + +(defun vc-git--rev-to-filename (rev) + "Return a historical file name for the file in REV." + (when rev + (setq rev (vc-git--rev-parse rev)) + (cadr (member rev (vc-git--shalist))))) + +(defun vc-git--rev-to-previous-rev (rev) + "Return the revision before REV according to historical file +name data." + (when rev + (setq rev (vc-git--rev-parse rev)) + (car (cddr (member rev (vc-git--shalist)))))) + (defun vc-git-find-revision (file rev buffer) (let* (process-file-side-effects (coding-system-for-read 'binary) (coding-system-for-write 'binary) (fullname - (let ((fn (vc-git--run-command-string - file "ls-files" "-z" "--full-name" "--"))) - ;; ls-files does not return anything when looking for a - ;; revision of a file that has been renamed or removed. - (if (string= fn "") - (file-relative-name file (vc-git-root default-directory)) - (substring fn 0 -1))))) + (or (vc-git--rev-to-filename rev) + (let ((fn (vc-git--run-command-string + file "ls-files" "-z" "--full-name" "--"))) + ;; ls-files does not return anything when looking for a + ;; revision of a file that has been renamed or removed. + (if (string= fn "") + (file-relative-name file (vc-git-root default-directory)) + (substring fn 0 -1)))))) (vc-git-command buffer 0 nil @@ -1182,6 +1241,7 @@ vc-git-print-log ;; "--follow" on directories or multiple files is broken ;; https://debbugs.gnu.org/cgi/bugreport.cgi?bug=8756 ;; https://debbugs.gnu.org/cgi/bugreport.cgi?bug=16422 + (vc-git--make-shalist buffer files start-revision limit) (list "--follow")) (when shortlog `("--graph" "--decorate" "--date=short" @@ -1423,6 +1483,13 @@ vc-git-diff (unless rev1 (setq rev1 "4b825dc642cb6eb9a060e54bf8d69288fbee4904")) (setq command "diff-index") (unless rev1 (setq rev1 "HEAD"))) + (let ((file1 (vc-git--rev-to-filename rev1)) + (file2 (vc-git--rev-to-filename rev2))) + (when (and file1 file2) + ;; Run diff from the repository root because our file names are + ;; relative to it + (setq default-directory (vc-git-root default-directory) + files (list file1 file2)))) (if vc-git-diff-switches (apply #'vc-git-command (or buffer "*vc-diff*") 1 ; bug#21969 @@ -1459,7 +1526,9 @@ vc-git-revision-completion-table (defun vc-git-annotate-command (file buf &optional rev) (vc-git--asciify-coding-system) - (let ((name (file-relative-name file))) + (let ((name (vc-git--rev-to-filename rev))) + (if name (setq default-directory (vc-git-root default-directory)) + (setq name (file-relative-name file))) (apply #'vc-git-command buf 'async nil "blame" "--date=short" (append (vc-switches 'git 'annotate) (list rev "--" name))))) @@ -1505,6 +1574,10 @@ vc-git-retrieve-tag ;;; MISCELLANEOUS +;; HACK: let log-view.el inform vc-git.el whether we are doing a +;; "whole changeset" diff or not. +(defvar git-log-view-diff-whole-changeset nil) + (defun vc-git-previous-revision (file rev) "Git-specific version of `vc-previous-revision'." (if file @@ -1520,11 +1593,13 @@ vc-git-previous-revision (point) (1- (point-max))))))) (or (vc-git-symbolic-commit prev-rev) prev-rev)) - ;; We used to use "^" here, but that fails on MS-Windows if git is - ;; invoked via a batch file, in which case cmd.exe strips the "^" - ;; because it is a special character for cmd which process-file - ;; does not (and cannot) quote. - (vc-git--rev-parse (concat rev "~1")))) + ;; Use historical data for the file if possible. + (or (and (not git-log-view-diff-whole-changeset) (vc-git--rev-to-previous-rev rev)) + ;; We used to use "^" here, but that fails on MS-Windows if git is + ;; invoked via a batch file, in which case cmd.exe strips the "^" + ;; because it is a special character for cmd which process-file + ;; does not (and cannot) quote. + (vc-git--rev-parse (concat rev "~1"))))) (defun vc-git--rev-parse (rev) (with-temp-buffer -- 2.17.1 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* bug#55871: Acknowledgement (27.1; vc-git.el log view 'a', 'f', 'd' do not work when following renames) 2022-06-10 17:31 ` bug#55871: Acknowledgement (27.1; vc-git.el log view 'a', 'f', 'd' do not work when following renames) Nicolás Ojeda Bär @ 2022-08-18 2:10 ` Dmitry Gutov 2022-09-06 10:56 ` bug#55871: 27.1; vc-git.el log view 'a', 'f', 'd' do not work when following renames Lars Ingebrigtsen 2022-12-11 23:02 ` bug#55871: Acknowledgement (27.1; vc-git.el log view 'a', 'f', 'd' do not work when following renames) Dmitry Gutov 0 siblings, 2 replies; 18+ messages in thread From: Dmitry Gutov @ 2022-08-18 2:10 UTC (permalink / raw) To: Nicolás Ojeda Bär, 55871 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. ^ permalink raw reply [flat|nested] 18+ messages in thread
* bug#55871: 27.1; vc-git.el log view 'a', 'f', 'd' do not work when following renames 2022-08-18 2:10 ` Dmitry Gutov @ 2022-09-06 10:56 ` Lars Ingebrigtsen 2022-09-06 12:12 ` Nicolás Ojeda Bär 2022-12-11 23:02 ` bug#55871: Acknowledgement (27.1; vc-git.el log view 'a', 'f', 'd' do not work when following renames) Dmitry Gutov 1 sibling, 1 reply; 18+ messages in thread From: Lars Ingebrigtsen @ 2022-09-06 10:56 UTC (permalink / raw) To: Dmitry Gutov; +Cc: 55871, Nicolás Ojeda Bär Dmitry Gutov <dgutov@yandex.ru> writes: > First of all, how do you feel about signing copyright assignment for > Emacs? This was a couple of weeks ago, but there wasn't any followup as far as I can tell. Nicolás, would you be OK with assigning copyright to the FSF? ^ permalink raw reply [flat|nested] 18+ messages in thread
* bug#55871: 27.1; vc-git.el log view 'a', 'f', 'd' do not work when following renames 2022-09-06 10:56 ` bug#55871: 27.1; vc-git.el log view 'a', 'f', 'd' do not work when following renames Lars Ingebrigtsen @ 2022-09-06 12:12 ` Nicolás Ojeda Bär 2022-09-06 12:13 ` Lars Ingebrigtsen 2022-12-03 2:02 ` Dmitry Gutov 0 siblings, 2 replies; 18+ messages in thread From: Nicolás Ojeda Bär @ 2022-09-06 12:12 UTC (permalink / raw) To: Lars Ingebrigtsen; +Cc: 55871, Dmitry Gutov Hello, Sorry for the delay in getting back to you. No problem with assigning copyright to FSF. I will reply to Dmity separately. Cheers, Nicolas On Tue, Sep 6, 2022 at 12:56 PM Lars Ingebrigtsen <larsi@gnus.org> wrote: > > Dmitry Gutov <dgutov@yandex.ru> writes: > > > First of all, how do you feel about signing copyright assignment for > > Emacs? > > This was a couple of weeks ago, but there wasn't any followup as far as > I can tell. > > Nicolás, would you be OK with assigning copyright to the FSF? > ^ permalink raw reply [flat|nested] 18+ messages in thread
* bug#55871: 27.1; vc-git.el log view 'a', 'f', 'd' do not work when following renames 2022-09-06 12:12 ` Nicolás Ojeda Bär @ 2022-09-06 12:13 ` Lars Ingebrigtsen 2022-12-03 2:02 ` Dmitry Gutov 1 sibling, 0 replies; 18+ messages in thread From: Lars Ingebrigtsen @ 2022-09-06 12:13 UTC (permalink / raw) To: Nicolás Ojeda Bär; +Cc: 55871, Dmitry Gutov Nicolás Ojeda Bär <n.oje.bar@gmail.com> writes: > Sorry for the delay in getting back to you. No problem with assigning > copyright to FSF. Great; here's the form to get started: Please email the following information to assign@gnu.org, and we will send you the assignment form for your past and future changes. Please use your full legal name (in ASCII characters) as the subject line of the message. ---------------------------------------------------------------------- REQUEST: SEND FORM FOR PAST AND FUTURE CHANGES [What is the name of the program or package you're contributing to?] Emacs [Did you copy any files or text written by someone else in these changes? Even if that material is free software, we need to know about it.] [Do you have an employer who might have a basis to claim to own your changes? Do you attend a school which might make such a claim?] [For the copyright registration, what country are you a citizen of?] [What year were you born?] [Please write your email address here.] [Please write your postal address here.] [Which files have you changed so far, and which new files have you written so far?] ^ permalink raw reply [flat|nested] 18+ messages in thread
* bug#55871: 27.1; vc-git.el log view 'a', 'f', 'd' do not work when following renames 2022-09-06 12:12 ` Nicolás Ojeda Bär 2022-09-06 12:13 ` Lars Ingebrigtsen @ 2022-12-03 2:02 ` Dmitry Gutov 1 sibling, 0 replies; 18+ messages in thread From: Dmitry Gutov @ 2022-12-03 2:02 UTC (permalink / raw) To: Nicolás Ojeda Bär, Lars Ingebrigtsen; +Cc: 55871 Hi Nicholas, On 06/09/2022 15:12, Nicolás Ojeda Bär wrote: > Sorry for the delay in getting back to you. No problem with assigning > copyright to FSF. > > I will reply to Dmity separately. How's progress with the copyright assignment or the additional reply? If you got the ball rolling, we can continue the discussion now. If you hit some problems (with your employer or etc), I was thinking of try an alternative patch based on the 2014 one (which has its copyright assigned). Or if maybe you have something newer to share, please do. ^ permalink raw reply [flat|nested] 18+ messages in thread
* bug#55871: Acknowledgement (27.1; vc-git.el log view 'a', 'f', 'd' do not work when following renames) 2022-08-18 2:10 ` Dmitry Gutov 2022-09-06 10:56 ` bug#55871: 27.1; vc-git.el log view 'a', 'f', 'd' do not work when following renames Lars Ingebrigtsen @ 2022-12-11 23:02 ` Dmitry Gutov 2022-12-12 16:44 ` Nicolás Ojeda Bär 1 sibling, 1 reply; 18+ messages in thread From: Dmitry Gutov @ 2022-12-11 23:02 UTC (permalink / raw) To: Nicolás Ojeda Bär, 55871 On 18/08/2022 05:10, Dmitry Gutov wrote: > 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. I've tried another idea: to pre-process the file's history and pass all historical file names to 'git log' inside vc-git-print-log. Unfortunately, that delays the appearance of the log significantly. In the Emacs repo that comes down to several seconds, which seems unacceptable. But that would fix both the problems with a/f/d and the bug described in the SO question above. Looking around for how other software deals with it, it seems GitHub has found a satisfactory solution which adds a new UI element with basically zero performance cost. At first it was implemented in a Chrome extension for it (https://github.com/jeffstieler/github-follow-extension), but then added to the core functionality this summer (https://github.blog/changelog/2022-06-06-view-commit-history-across-file-renames-and-moves/). This gif shows the workflow: https://i0.wp.com/user-images.githubusercontent.com/4021812/171795153-4f327a04-eb27-4d46-acb1-73d2e82ce4c5.gif?ssl=1 We should be able to do something similar. Step 1: Drop the '--follow' argument in all cases. Step 2: After the log is finished printing, we detect somehow that the last commit was a rename one. Perhaps using an additional process call, or perhaps by adding some output to the process which we'll hide through font-lock or process filter. When it is a rename, we print a message at the end, saying the file has been renamed. And a button saying e.g. "Print Previous Log", which would print the history for the previous name. That history should also include the missing commits from the SO question. Not sure how to deal with duplicating file names best (like etc/NEWS has been the name of many files in the Emacs repo): either limiting the first revision to start from -- but that keep bring back the missing commit problem, oh well -- or some other way. Can't exactly check what GitHub is doing, because they don't actually provide this for NEWS.24, guess because it was not a straight rename: https://github.com/emacs-mirror/emacs/commits/master/etc/NEWS.24 But git log -M50% -C --stat 5f8947c7007d1d8 -n 1 at least detects it as a copy if not a rename. Guess they didn't adopt the whole follow-renames logic, and we can do better. I don't have any code to show, but it shouldn't require too many changes. ^ permalink raw reply [flat|nested] 18+ messages in thread
* bug#55871: Acknowledgement (27.1; vc-git.el log view 'a', 'f', 'd' do not work when following renames) 2022-12-11 23:02 ` bug#55871: Acknowledgement (27.1; vc-git.el log view 'a', 'f', 'd' do not work when following renames) Dmitry Gutov @ 2022-12-12 16:44 ` Nicolás Ojeda Bär 2022-12-13 1:23 ` Dmitry Gutov 0 siblings, 1 reply; 18+ messages in thread From: Nicolás Ojeda Bär @ 2022-12-12 16:44 UTC (permalink / raw) To: Dmitry Gutov; +Cc: 55871 Dear Dmitry, Apologies for the delay in getting back to you; I have been submerged in my day job and haven't had any time to look at the code again (the copyright assignment is done, by the way). I read your message, and I think immitating GitHub's UI is a great idea, as it liberates us from having to do any kind of "pretreatment" of the git log (which can be expensive for large repos). We should focus in this direction to fix this issue. If I get some spare time over the holidays I will try to take a look (sadly, I cannot promise that will be the case...). Cheers, Nicolas On Mon, Dec 12, 2022 at 12:02 AM Dmitry Gutov <dgutov@yandex.ru> wrote: > > On 18/08/2022 05:10, Dmitry Gutov wrote: > > 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. > > I've tried another idea: to pre-process the file's history and pass all > historical file names to 'git log' inside vc-git-print-log. > > Unfortunately, that delays the appearance of the log significantly. In > the Emacs repo that comes down to several seconds, which seems > unacceptable. But that would fix both the problems with a/f/d and the > bug described in the SO question above. > > Looking around for how other software deals with it, it seems GitHub has > found a satisfactory solution which adds a new UI element with basically > zero performance cost. > > At first it was implemented in a Chrome extension for it > (https://github.com/jeffstieler/github-follow-extension), but then added > to the core functionality this summer > (https://github.blog/changelog/2022-06-06-view-commit-history-across-file-renames-and-moves/). > > This gif shows the workflow: > https://i0.wp.com/user-images.githubusercontent.com/4021812/171795153-4f327a04-eb27-4d46-acb1-73d2e82ce4c5.gif?ssl=1 > > We should be able to do something similar. > > Step 1: Drop the '--follow' argument in all cases. > > Step 2: After the log is finished printing, we detect somehow that the > last commit was a rename one. Perhaps using an additional process call, > or perhaps by adding some output to the process which we'll hide through > font-lock or process filter. When it is a rename, we print a message at > the end, saying the file has been renamed. And a button saying e.g. > "Print Previous Log", which would print the history for the previous > name. That history should also include the missing commits from the SO > question. > > Not sure how to deal with duplicating file names best (like etc/NEWS has > been the name of many files in the Emacs repo): either limiting the > first revision to start from -- but that keep bring back the missing > commit problem, oh well -- or some other way. > > Can't exactly check what GitHub is doing, because they don't actually > provide this for NEWS.24, guess because it was not a straight rename: > https://github.com/emacs-mirror/emacs/commits/master/etc/NEWS.24 > > But > > git log -M50% -C --stat 5f8947c7007d1d8 -n 1 > > at least detects it as a copy if not a rename. Guess they didn't adopt > the whole follow-renames logic, and we can do better. > > I don't have any code to show, but it shouldn't require too many changes. ^ permalink raw reply [flat|nested] 18+ messages in thread
* bug#55871: Acknowledgement (27.1; vc-git.el log view 'a', 'f', 'd' do not work when following renames) 2022-12-12 16:44 ` Nicolás Ojeda Bär @ 2022-12-13 1:23 ` Dmitry Gutov 2023-12-14 0:52 ` Dmitry Gutov 0 siblings, 1 reply; 18+ messages in thread From: Dmitry Gutov @ 2022-12-13 1:23 UTC (permalink / raw) To: Nicolás Ojeda Bär; +Cc: 55871 Dear Nicolas, On 12/12/2022 18:44, Nicolás Ojeda Bär wrote: > Apologies for the delay in getting back to you; I have been submerged > in my day job and haven't had any time to look at the code again (the > copyright assignment is done, by the way). Good to hear about the copyright assignment. No problem about delayed replies, I first of all wanted to experiment with an alternative and see how others deal with it. > I read your message, and I think immitating GitHub's UI is a great > idea, as it liberates us from having to do any kind of "pretreatment" > of the git log (which can be expensive for large repos). Indeed. I suppose we'll lose out in some more complex cases (e.g. if --follow tracks copies, it might track the cases when several files had been copied into one, including when that action spanned several commits; and thus --follow could show the history of each such file), but we should win in the most common cases (single-file copies and renames) OOtB, which we don't have any good support for still. > We should focus in this direction to fix this issue. If I get some > spare time over the holidays I will try to take a look (sadly, I > cannot promise that will be the case...). We won't be able to put the improvement into Emacs 29 anyway (the release branch has been cut, it's now bugfix-only), so there is no hurry. Let's see who gets to this first. If you wanted to finish up your patch instead, I'm not going to say no either. But GitHub's approach seems like it should require less (and less complex) code. Best, Dmitry. ^ permalink raw reply [flat|nested] 18+ messages in thread
* bug#55871: Acknowledgement (27.1; vc-git.el log view 'a', 'f', 'd' do not work when following renames) 2022-12-13 1:23 ` Dmitry Gutov @ 2023-12-14 0:52 ` Dmitry Gutov 2023-12-14 1:23 ` Dmitry Gutov 0 siblings, 1 reply; 18+ messages in thread From: Dmitry Gutov @ 2023-12-14 0:52 UTC (permalink / raw) To: Nicolás Ojeda Bär; +Cc: 55871 [-- Attachment #1: Type: text/plain, Size: 1902 bytes --] On 13/12/2022 03:23, Dmitry Gutov wrote: >> I read your message, and I think immitating GitHub's UI is a great >> idea, as it liberates us from having to do any kind of "pretreatment" >> of the git log (which can be expensive for large repos). > > Indeed. I suppose we'll lose out in some more complex cases (e.g. if > --follow tracks copies, it might track the cases when several files had > been copied into one, including when that action spanned several > commits; and thus --follow could show the history of each such file), > but we should win in the most common cases (single-file copies and > renames) OOtB, which we don't have any good support for still. > >> We should focus in this direction to fix this issue. If I get some >> spare time over the holidays I will try to take a look (sadly, I >> cannot promise that will be the case...). > > We won't be able to put the improvement into Emacs 29 anyway (the > release branch has been cut, it's now bugfix-only), so there is no hurry. > > Let's see who gets to this first. If you wanted to finish up your patch > instead, I'm not going to say no either. But GitHub's approach seems > like it should require less (and less complex) code. Attached is the implementation for this alternative approach. It's Git-only, but otherwise seems to function well (with potential for future additions). The look of the message and the button could use some work, but this is the best I came up thus far. As far as testing, it allowed following all files in Emacs's repo (that I have tried) to their original creation. Including etc/NEWS.29, and most other NEWS.*, with NEWS.28 being an exception (apparently because on that occasion NEWS was truncated before NEWS.28 was created). Also important: set vc-git-print-log-follow to nil. Otherwise logs don't end on renames, and this feature doesn't get a chance to work. Cheers, Dmitry. [-- Attachment #2: vc-git-file-name-changes.diff --] [-- Type: text/x-patch, Size: 7123 bytes --] diff --git a/etc/NEWS b/etc/NEWS index 1ff2f8a149f..4dd11c99927 100644 --- a/etc/NEWS +++ b/etc/NEWS @@ -457,6 +457,14 @@ With this value only the revision number is displayed on the mode-line. *** Obsolete command 'vc-switch-backend' re-added as 'vc-change-backend'. The command was previously obsoleted and unbound in Emacs 28. +*** Support for viewing file change history across renames. +When a fileset's VC change history ends at a rename, we now print the +old name(s) and a button which jumps to their history. Only supported +with Git at the moment. + +*** New option 'vc-git-file-name-changes-switches'. +It allows tweaking the thresholds for rename and copy detection. + ** Diff mode +++ diff --git a/lisp/vc/vc-git.el b/lisp/vc/vc-git.el index 2e057ecfaa7..e2c2ed5c79c 100644 --- a/lisp/vc/vc-git.el +++ b/lisp/vc/vc-git.el @@ -89,6 +89,7 @@ ;; - make-version-backups-p (file) NOT NEEDED ;; - previous-revision (file rev) OK ;; - next-revision (file rev) OK +;; - file-name-changes (rev) OK ;; - check-headers () COULD BE SUPPORTED ;; - delete-file (file) OK ;; - rename-file (old new) OK @@ -152,6 +153,20 @@ vc-git-shortlog-switches (repeat :tag "Argument List" :value ("") string)) :version "30.1") +;; XXX: (setq vc-git-log-switches '("--simplify-merges")) can also +;; create fuller history when using this feature. Not sure why. +(defcustom vc-git-file-name-changes-switches '("-M" "-C") + "String or list of string to pass to Git when finding previous names. + +This option should usually at least contain '-M'. You can adjust +the flags to change the similarity thresholds (default 50%). Or +add `--find-copies-harder' (slower in large projects, since it +uses a full scan)." + :type '(choice (const :tag "None" nil) + (string :tag "Argument String") + (repeat :tag "Argument List" :value ("") string)) + :version "30.1") + (defcustom vc-git-resolve-conflicts t "When non-nil, mark conflicted file as resolved upon saving. That is performed after all conflict markers in it have been @@ -1239,6 +1254,30 @@ vc-git-find-revision nil "cat-file" "blob" (concat (if rev rev "HEAD") ":" fullname)))) +(defun vc-git-file-name-changes (rev) + (with-temp-buffer + (let ((root (vc-git-root default-directory))) + (apply #'vc-git-command (current-buffer) t nil + "diff" + "--name-status" + "--diff-filter=ADCR" + (concat rev "^") rev + (vc-switches 'git 'file-name-changes)) + (let (res) + (goto-char (point-min)) + (while (re-search-forward "^\\([CMR]\\)[0-9]*\t\\([^\n\t]+\\)\\(?:\t\\([^\n\t]+\\)\\)?" nil t) + (pcase (match-string 1) + ("A" (push (cons nil (match-string 2)) res)) + ("D" (push (cons (match-string 2) nil) res)) + ((or "C" "R") (push (cons (match-string 2) (match-string 3)) res)) + ;; ("M" (push (cons (match-string 1) (match-string 1)) res)) + )) + (mapc (lambda (c) + (if (car c) (setcar c (expand-file-name (car c) root))) + (if (cdr c) (setcdr c (expand-file-name (cdr c) root)))) + res) + (nreverse res))))) + (defun vc-git-find-ignore-file (file) "Return the git ignore file that controls FILE." (expand-file-name ".gitignore" @@ -1416,7 +1455,15 @@ vc-git-clone ;; Long explanation here: ;; https://stackoverflow.com/questions/46487476/git-log-follow-graph-skips-commits (defcustom vc-git-print-log-follow nil - "If true, follow renames in Git logs for a single file." + "If true, use the flag `--follow' when producing single file logs. + +It will make the printed log automatically follow the renames. +The downsides is that the log produced this way may omit +certain (merge) commits, and that `log-view-diff' fails on +commits that used the previous name, in that log buffer. + +When this variable is nil, and the log ends with a rename, we +print a button that shows the log for the previous name." :type 'boolean :version "26.1") diff --git a/lisp/vc/vc.el b/lisp/vc/vc.el index 958929fe4c6..e626d72d59a 100644 --- a/lisp/vc/vc.el +++ b/lisp/vc/vc.el @@ -517,6 +517,13 @@ ;; Return the revision number that precedes REV for FILE, or nil if no such ;; revision exists. ;; +;; - file-name-changes (rev) +;; +;; Return the list of pairs with changes in file names in REV. When +;; a file was added, it should be a cons with nil car. When +;; deleted, a cons with nil cdr. When copied or renamed, a cons +;; with the source name as car and destination name as cdr. +;; ;; - next-revision (file rev) ;; ;; Return the revision number that follows REV for FILE, or nil if no such @@ -2695,9 +2702,42 @@ vc-print-log-setup-buttons (goto-char (point-min)) (while (re-search-forward log-view-message-re nil t) (cl-incf entries)) - ;; If we got fewer entries than we asked for, then displaying - ;; the "more" buttons isn't useful. - (when (>= entries limit) + (if (< entries limit) + ;; The log has been printed in full. Perhaps it started + ;; with a copy or rename? + (let* ((last-revision (log-view-current-tag (point-max))) + ;; Could skip this when vc-git-print-log-follow = t. + (name-changes + (condition-case nil + (vc-call-backend log-view-vc-backend + 'file-name-changes last-revision) + (vc-not-supported nil))) + (matching-changes + (cl-delete-if-not (lambda (f) (member f log-view-vc-fileset)) + name-changes :key #'cdr)) + (old-names (mapcar #'car matching-changes)) + (relatives (mapcar #'file-relative-name old-names))) + (when old-names + (goto-char (point-max)) + (insert "\n") + (insert + (format + "Renamed from %s" + (mapconcat (lambda (s) + (propertize s 'font-lock-face + 'log-view-file)) + relatives ", ")) + " ") + ;; TODO: Also print a "Next log" button above the buffer + ;; created by this button to be able to go back quickly. + (insert-text-button + "View log" + 'action (lambda (&rest _ignore) + (vc-print-log-internal log-view-vc-backend old-names + last-revision nil limit)) + 'help-echo + "Show the log for the file name(s) before the rename"))) + ;; Perhaps there are more entries in the log. (goto-char (point-max)) (insert "\n") (insert-text-button ^ permalink raw reply related [flat|nested] 18+ messages in thread
* bug#55871: Acknowledgement (27.1; vc-git.el log view 'a', 'f', 'd' do not work when following renames) 2023-12-14 0:52 ` Dmitry Gutov @ 2023-12-14 1:23 ` Dmitry Gutov 2023-12-15 2:01 ` Dmitry Gutov 0 siblings, 1 reply; 18+ messages in thread From: Dmitry Gutov @ 2023-12-14 1:23 UTC (permalink / raw) To: Nicolás Ojeda Bär; +Cc: 55871 On 14/12/2023 02:52, Dmitry Gutov wrote: > but otherwise seems to function well (with potential for future additions) To clarify: this version only makes 'd' work (not 'f' or 'a'), but the other two are fixed more easily. ^ permalink raw reply [flat|nested] 18+ messages in thread
* bug#55871: Acknowledgement (27.1; vc-git.el log view 'a', 'f', 'd' do not work when following renames) 2023-12-14 1:23 ` Dmitry Gutov @ 2023-12-15 2:01 ` Dmitry Gutov 2023-12-15 13:05 ` Eli Zaretskii 0 siblings, 1 reply; 18+ messages in thread From: Dmitry Gutov @ 2023-12-15 2:01 UTC (permalink / raw) To: Nicolás Ojeda Bär; +Cc: 55871 [-- Attachment #1: Type: text/plain, Size: 326 bytes --] On 14/12/2023 03:23, Dmitry Gutov wrote: > On 14/12/2023 02:52, Dmitry Gutov wrote: >> but otherwise seems to function well (with potential for future >> additions) > > To clarify: this version only makes 'd' work (not 'f' or 'a'), but the > other two are fixed more easily. Attached is the new revision with combined fix. [-- Attachment #2: vc-git-file-name-changes-v2.diff --] [-- Type: text/x-patch, Size: 8153 bytes --] diff --git a/etc/NEWS b/etc/NEWS index 1ff2f8a149f..4dd11c99927 100644 --- a/etc/NEWS +++ b/etc/NEWS @@ -457,6 +457,14 @@ With this value only the revision number is displayed on the mode-line. *** Obsolete command 'vc-switch-backend' re-added as 'vc-change-backend'. The command was previously obsoleted and unbound in Emacs 28. +*** Support for viewing file change history across renames. +When a fileset's VC change history ends at a rename, we now print the +old name(s) and a button which jumps to their history. Only supported +with Git at the moment. + +*** New option 'vc-git-file-name-changes-switches'. +It allows tweaking the thresholds for rename and copy detection. + ** Diff mode +++ diff --git a/lisp/vc/log-view.el b/lisp/vc/log-view.el index af24fcfd398..6c3abd15d8d 100644 --- a/lisp/vc/log-view.el +++ b/lisp/vc/log-view.el @@ -516,7 +516,8 @@ log-view-find-revision (switch-to-buffer (vc-find-revision (if log-view-per-file-logs (log-view-current-file) (car log-view-vc-fileset)) - (log-view-current-tag))))) + (log-view-current-tag) + log-view-vc-backend)))) (defun log-view-extract-comment () @@ -562,7 +563,8 @@ log-view-annotate-version (vc-annotate (if log-view-per-file-logs (log-view-current-file) (car log-view-vc-fileset)) - (log-view-current-tag)))) + (log-view-current-tag) + nil nil nil log-view-vc-backend))) ;; ;; diff diff --git a/lisp/vc/vc-git.el b/lisp/vc/vc-git.el index 2e057ecfaa7..f1d0ffd9b44 100644 --- a/lisp/vc/vc-git.el +++ b/lisp/vc/vc-git.el @@ -89,6 +89,7 @@ ;; - make-version-backups-p (file) NOT NEEDED ;; - previous-revision (file rev) OK ;; - next-revision (file rev) OK +;; - file-name-changes (rev) OK ;; - check-headers () COULD BE SUPPORTED ;; - delete-file (file) OK ;; - rename-file (old new) OK @@ -152,6 +153,20 @@ vc-git-shortlog-switches (repeat :tag "Argument List" :value ("") string)) :version "30.1") +;; XXX: (setq vc-git-log-switches '("--simplify-merges")) can also +;; create fuller history when using this feature. Not sure why. +(defcustom vc-git-file-name-changes-switches '("-M" "-C") + "String or list of string to pass to Git when finding previous names. + +This option should usually at least contain '-M'. You can adjust +the flags to change the similarity thresholds (default 50%). Or +add `--find-copies-harder' (slower in large projects, since it +uses a full scan)." + :type '(choice (const :tag "None" nil) + (string :tag "Argument String") + (repeat :tag "Argument List" :value ("") string)) + :version "30.1") + (defcustom vc-git-resolve-conflicts t "When non-nil, mark conflicted file as resolved upon saving. That is performed after all conflict markers in it have been @@ -1239,6 +1254,30 @@ vc-git-find-revision nil "cat-file" "blob" (concat (if rev rev "HEAD") ":" fullname)))) +(defun vc-git-file-name-changes (rev) + (with-temp-buffer + (let ((root (vc-git-root default-directory))) + (apply #'vc-git-command (current-buffer) t nil + "diff" + "--name-status" + "--diff-filter=ADCR" + (concat rev "^") rev + (vc-switches 'git 'file-name-changes)) + (let (res) + (goto-char (point-min)) + (while (re-search-forward "^\\([ADCR]\\)[0-9]*\t\\([^\n\t]+\\)\\(?:\t\\([^\n\t]+\\)\\)?" nil t) + (pcase (match-string 1) + ("A" (push (cons nil (match-string 2)) res)) + ("D" (push (cons (match-string 2) nil) res)) + ((or "C" "R") (push (cons (match-string 2) (match-string 3)) res)) + ;; ("M" (push (cons (match-string 1) (match-string 1)) res)) + )) + (mapc (lambda (c) + (if (car c) (setcar c (expand-file-name (car c) root))) + (if (cdr c) (setcdr c (expand-file-name (cdr c) root)))) + res) + (nreverse res))))) + (defun vc-git-find-ignore-file (file) "Return the git ignore file that controls FILE." (expand-file-name ".gitignore" @@ -1416,7 +1455,15 @@ vc-git-clone ;; Long explanation here: ;; https://stackoverflow.com/questions/46487476/git-log-follow-graph-skips-commits (defcustom vc-git-print-log-follow nil - "If true, follow renames in Git logs for a single file." + "If true, use the flag `--follow' when producing single file logs. + +It will make the printed log automatically follow the renames. +The downsides is that the log produced this way may omit +certain (merge) commits, and that `log-view-diff' fails on +commits that used the previous name, in that log buffer. + +When this variable is nil, and the log ends with a rename, we +print a button that shows the log for the previous name." :type 'boolean :version "26.1") diff --git a/lisp/vc/vc.el b/lisp/vc/vc.el index 958929fe4c6..fab454994e1 100644 --- a/lisp/vc/vc.el +++ b/lisp/vc/vc.el @@ -517,6 +517,13 @@ ;; Return the revision number that precedes REV for FILE, or nil if no such ;; revision exists. ;; +;; - file-name-changes (rev) +;; +;; Return the list of pairs with changes in file names in REV. When +;; a file was added, it should be a cons with nil car. When +;; deleted, a cons with nil cdr. When copied or renamed, a cons +;; with the source name as car and destination name as cdr. +;; ;; - next-revision (file rev) ;; ;; Return the revision number that follows REV for FILE, or nil if no such @@ -2695,9 +2702,46 @@ vc-print-log-setup-buttons (goto-char (point-min)) (while (re-search-forward log-view-message-re nil t) (cl-incf entries)) - ;; If we got fewer entries than we asked for, then displaying - ;; the "more" buttons isn't useful. - (when (>= entries limit) + (if (< entries limit) + ;; The log has been printed in full. Perhaps it started + ;; with a copy or rename? + (let* ((last-revision (log-view-current-tag (point-max))) + ;; XXX: Could skip this when vc-git-print-log-follow = t. + (name-changes + (condition-case nil + (vc-call-backend log-view-vc-backend + 'file-name-changes last-revision) + (vc-not-supported nil))) + (matching-changes + (cl-delete-if-not (lambda (f) (member f log-view-vc-fileset)) + name-changes :key #'cdr)) + (old-names (mapcar #'car matching-changes)) + (relatives (mapcar #'file-relative-name old-names))) + (when old-names + (goto-char (point-max)) + (insert "\n") + (insert + (format + "Renamed from %s" + (mapconcat (lambda (s) + (propertize s 'font-lock-face + 'log-view-file)) + relatives ", ")) + " ") + ;; TODO: Also print a different button somewhere in the + ;; created buffer to be able to go back easily. (There + ;; are different ways to do that.) + (insert-text-button + "View log" + 'action (lambda (&rest _ignore) + (let ((backend log-view-vc-backend)) + (with-current-buffer vc-parent-buffer + ;; To set up parent buffer in the new viewer. + (vc-print-log-internal backend old-names + last-revision nil limit)))) + 'help-echo + "Show the log for the file name(s) before the rename"))) + ;; Perhaps there are more entries in the log. (goto-char (point-max)) (insert "\n") (insert-text-button ^ permalink raw reply related [flat|nested] 18+ messages in thread
* bug#55871: Acknowledgement (27.1; vc-git.el log view 'a', 'f', 'd' do not work when following renames) 2023-12-15 2:01 ` Dmitry Gutov @ 2023-12-15 13:05 ` Eli Zaretskii 2023-12-15 14:39 ` Dmitry Gutov 0 siblings, 1 reply; 18+ messages in thread From: Eli Zaretskii @ 2023-12-15 13:05 UTC (permalink / raw) To: Dmitry Gutov; +Cc: 55871, n.oje.bar > Cc: 55871@debbugs.gnu.org > Date: Fri, 15 Dec 2023 04:01:59 +0200 > From: Dmitry Gutov <dgutov@yandex.ru> > > On 14/12/2023 03:23, Dmitry Gutov wrote: > > On 14/12/2023 02:52, Dmitry Gutov wrote: > >> but otherwise seems to function well (with potential for future > >> additions) > > > > To clarify: this version only makes 'd' work (not 'f' or 'a'), but the > > other two are fixed more easily. > > Attached is the new revision with combined fix. FYI: 'd' and 'f' work with bzr without any changes. 'a' doesn't work (evidently, "bzr annotate -r REVISION FILE" doesn't work when FILE did not exist in REVISION, but was renamed by a later revision, and one needs to run "bzr status -Sr REVISION" and look for the "renamed" report in the result, which will then provide the previous name). (FTR: I used src/unexcoff.c file to test this.) > +*** Support for viewing file change history across renames. > +When a fileset's VC change history ends at a rename, we now print the > +old name(s) and a button which jumps to their history. Only supported > +with Git at the moment. I think this should at least tell that for files under Bazaar, the VC change history will always include the renames. Looks like Mercurial is in the same department as Git? If so, I think the text should say that this is not supported for Mercurial yet, and that Bazaar shows the entire history, including renames, by default. Or something like that. I also think this entry should mention the relevant VC commands ("C-x v l" and what else?), since otherwise "VC change history" is not concrete enough to tell users which command(s) is/are affected. Thanks. ^ permalink raw reply [flat|nested] 18+ messages in thread
* bug#55871: Acknowledgement (27.1; vc-git.el log view 'a', 'f', 'd' do not work when following renames) 2023-12-15 13:05 ` Eli Zaretskii @ 2023-12-15 14:39 ` Dmitry Gutov 2023-12-15 15:10 ` Eli Zaretskii 0 siblings, 1 reply; 18+ messages in thread From: Dmitry Gutov @ 2023-12-15 14:39 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 55871, n.oje.bar On 15/12/2023 15:05, Eli Zaretskii wrote: >> Cc: 55871@debbugs.gnu.org >> Date: Fri, 15 Dec 2023 04:01:59 +0200 >> From: Dmitry Gutov <dgutov@yandex.ru> >> >> On 14/12/2023 03:23, Dmitry Gutov wrote: >>> On 14/12/2023 02:52, Dmitry Gutov wrote: >>>> but otherwise seems to function well (with potential for future >>>> additions) >>> >>> To clarify: this version only makes 'd' work (not 'f' or 'a'), but the >>> other two are fixed more easily. >> >> Attached is the new revision with combined fix. > > FYI: 'd' and 'f' work with bzr without any changes. To my understanding, Bazaar doesn't really exist in this day and age, so should we pay extra attention to it in this NEWS entry? We could say that the problem is relevant to Git and Hg, and the current solution only helps Git. I'm not sure what's the best phrasing, however, which won't bloat the NEWS entry too much. Though by the time 30 is released someone might implement the 'file-name-changes' handler for Hg too: it might be easy enough with the rest of the solution already in place. One will have to see whether Hg has any special caveats, though, like the one you noted for Bzr below. > 'a' doesn't work > (evidently, "bzr annotate -r REVISION FILE" doesn't work when FILE did > not exist in REVISION, but was renamed by a later revision, and one > needs to run "bzr status -Sr REVISION" and look for the "renamed" > report in the result, which will then provide the previous name). But when we're asking for 'annotate' for a file in some old revision (under old name), it won't be the same revision where it had been renamed, 99% of the time. > (FTR: I used src/unexcoff.c file to test this.) > >> +*** Support for viewing file change history across renames. >> +When a fileset's VC change history ends at a rename, we now print the >> +old name(s) and a button which jumps to their history. Only supported >> +with Git at the moment. > > I think this should at least tell that for files under Bazaar, the VC > change history will always include the renames. Looks like Mercurial > is in the same department as Git? More or less, yes, here's an even older bug report: https://debbugs.gnu.org/13004 > If so, I think the text should say > that this is not supported for Mercurial yet, and that Bazaar shows > the entire history, including renames, by default. Or something like > that. I don't want to make it a sticking point, but according to the wiki entry Monotone also tracks renames. We won't be mentioning it here, will we? I don't think we're going to persuade anybody new to use Bazaar here, and those who currently use it, probably don't pay this issue (the lack of it) much attention. > I also think this entry should mention the relevant VC commands > ("C-x v l" and what else?), since otherwise "VC change history" > is not concrete enough to tell users which command(s) is/are affected. Ok. Just 'C-x v l', with one or more files selected. The remaining 2 vc-print-* commands print the history for all files, so all renames are naturally included. One cannot use 'f' or 'a' on their output, though. ^ permalink raw reply [flat|nested] 18+ messages in thread
* bug#55871: Acknowledgement (27.1; vc-git.el log view 'a', 'f', 'd' do not work when following renames) 2023-12-15 14:39 ` Dmitry Gutov @ 2023-12-15 15:10 ` Eli Zaretskii 2023-12-15 20:45 ` Dmitry Gutov 0 siblings, 1 reply; 18+ messages in thread From: Eli Zaretskii @ 2023-12-15 15:10 UTC (permalink / raw) To: Dmitry Gutov; +Cc: 55871, n.oje.bar > Date: Fri, 15 Dec 2023 16:39:04 +0200 > Cc: 55871@debbugs.gnu.org, n.oje.bar@gmail.com > From: Dmitry Gutov <dgutov@yandex.ru> > > > FYI: 'd' and 'f' work with bzr without any changes. > > To my understanding, Bazaar doesn't really exist in this day and age, so > should we pay extra attention to it in this NEWS entry? Bazaar is still being developed, but I don't know how important it is nowadays. I do use it here, FWIW (and keep the old repository under Bazaar, from before the switch, for the rare occasion I need to look up or try something there). > We could say that the problem is relevant to Git and Hg, and the current > solution only helps Git. I'm not sure what's the best phrasing, however, > which won't bloat the NEWS entry too much. That'd be fine, and the wording could just use what you say above. > > 'a' doesn't work > > (evidently, "bzr annotate -r REVISION FILE" doesn't work when FILE did > > not exist in REVISION, but was renamed by a later revision, and one > > needs to run "bzr status -Sr REVISION" and look for the "renamed" > > report in the result, which will then provide the previous name). > > But when we're asking for 'annotate' for a file in some old revision > (under old name), it won't be the same revision where it had been > renamed, 99% of the time. It doesn't matter. The above-mentioned bzr command will show the telltale "RM old => new" status. Keep in mind that "-r REVISION" in bzr means "since REVISION till the current head". > > (FTR: I used src/unexcoff.c file to test this.) > > > >> +*** Support for viewing file change history across renames. > >> +When a fileset's VC change history ends at a rename, we now print the > >> +old name(s) and a button which jumps to their history. Only supported > >> +with Git at the moment. > > > > I think this should at least tell that for files under Bazaar, the VC > > change history will always include the renames. Looks like Mercurial > > is in the same department as Git? > > More or less, yes, here's an even older bug report: > https://debbugs.gnu.org/13004 > > > If so, I think the text should say > > that this is not supported for Mercurial yet, and that Bazaar shows > > the entire history, including renames, by default. Or something like > > that. > > I don't want to make it a sticking point, but according to the wiki > entry Monotone also tracks renames. We won't be mentioning it here, will we? vc-monotone is not part of Emacs, right? It is okay to say "is not yet supported for Hg". ^ permalink raw reply [flat|nested] 18+ messages in thread
* bug#55871: Acknowledgement (27.1; vc-git.el log view 'a', 'f', 'd' do not work when following renames) 2023-12-15 15:10 ` Eli Zaretskii @ 2023-12-15 20:45 ` Dmitry Gutov 2023-12-16 7:21 ` Eli Zaretskii 0 siblings, 1 reply; 18+ messages in thread From: Dmitry Gutov @ 2023-12-15 20:45 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 55871-done, n.oje.bar Version: 30.1 On 15/12/2023 17:10, Eli Zaretskii wrote: >> Date: Fri, 15 Dec 2023 16:39:04 +0200 >> Cc: 55871@debbugs.gnu.org, n.oje.bar@gmail.com >> From: Dmitry Gutov <dgutov@yandex.ru> >> >>> FYI: 'd' and 'f' work with bzr without any changes. >> >> To my understanding, Bazaar doesn't really exist in this day and age, so >> should we pay extra attention to it in this NEWS entry? > > Bazaar is still being developed, but I don't know how important it is > nowadays. I do use it here, FWIW (and keep the old repository under > Bazaar, from before the switch, for the rare occasion I need to look > up or try something there). I'm hoping this change will make it easier to investigate old changesets, so you won't need to keep a separate repository (I do have several worktrees for Emacs around, but they're all managed by Git). >> We could say that the problem is relevant to Git and Hg, and the current >> solution only helps Git. I'm not sure what's the best phrasing, however, >> which won't bloat the NEWS entry too much. > > That'd be fine, and the wording could just use what you say above. Ah well, while investigating what Hg does and does not, I ended up implementing the change for it as well. Problem is, while it "natively tracks renames", it likewise requires a "--follow" flag, which is only supported in "hg log" but not "hg diff". So we just as well have to dig around to find what were the previous names. That's what the new vc-hg-file-name-changes does, except in a less reliable way than the Git solution (WRT odd file names). >>> 'a' doesn't work >>> (evidently, "bzr annotate -r REVISION FILE" doesn't work when FILE did >>> not exist in REVISION, but was renamed by a later revision, and one >>> needs to run "bzr status -Sr REVISION" and look for the "renamed" >>> report in the result, which will then provide the previous name). >> >> But when we're asking for 'annotate' for a file in some old revision >> (under old name), it won't be the same revision where it had been >> renamed, 99% of the time. > > It doesn't matter. The above-mentioned bzr command will show the > telltale "RM old => new" status. Keep in mind that "-r REVISION" in > bzr means "since REVISION till the current head". Ah, you mean some code will need to search through the changes up until revision to find all renames anyway? I suppose that could be implemented in vc-bzr-annotate (independent of this changeset). >>> (FTR: I used src/unexcoff.c file to test this.) >>> >>>> +*** Support for viewing file change history across renames. >>>> +When a fileset's VC change history ends at a rename, we now print the >>>> +old name(s) and a button which jumps to their history. Only supported >>>> +with Git at the moment. >>> >>> I think this should at least tell that for files under Bazaar, the VC >>> change history will always include the renames. Looks like Mercurial >>> is in the same department as Git? >> >> More or less, yes, here's an even older bug report: >> https://debbugs.gnu.org/13004 >> >>> If so, I think the text should say >>> that this is not supported for Mercurial yet, and that Bazaar shows >>> the entire history, including renames, by default. Or something like >>> that. >> >> I don't want to make it a sticking point, but according to the wiki >> entry Monotone also tracks renames. We won't be mentioning it here, will we? > > vc-monotone is not part of Emacs, right? We do have vc-mtn.el in lisp/obsolete. Anyway... I've pushed the change to master (5b80894d0a7) to encourage wider testing. There are some potential follow-ups remaining (e.g. the question whether the "old" log should start with the exact same revision as the one that the "new" log ended at -- that makes a difference in some edge cases), but they should be small enough. Either way, looking forward to the feedback (and if you want to do any edits to the NEWS entry, please go ahead). ^ permalink raw reply [flat|nested] 18+ messages in thread
* bug#55871: Acknowledgement (27.1; vc-git.el log view 'a', 'f', 'd' do not work when following renames) 2023-12-15 20:45 ` Dmitry Gutov @ 2023-12-16 7:21 ` Eli Zaretskii 0 siblings, 0 replies; 18+ messages in thread From: Eli Zaretskii @ 2023-12-16 7:21 UTC (permalink / raw) To: Dmitry Gutov; +Cc: 55871-done, n.oje.bar > Date: Fri, 15 Dec 2023 22:45:24 +0200 > Cc: 55871-done@debbugs.gnu.org, n.oje.bar@gmail.com > From: Dmitry Gutov <dgutov@yandex.ru> > > >> We could say that the problem is relevant to Git and Hg, and the current > >> solution only helps Git. I'm not sure what's the best phrasing, however, > >> which won't bloat the NEWS entry too much. > > > > That'd be fine, and the wording could just use what you say above. > > Ah well, while investigating what Hg does and does not, I ended up > implementing the change for it as well. Thanks. > >>> 'a' doesn't work > >>> (evidently, "bzr annotate -r REVISION FILE" doesn't work when FILE did > >>> not exist in REVISION, but was renamed by a later revision, and one > >>> needs to run "bzr status -Sr REVISION" and look for the "renamed" > >>> report in the result, which will then provide the previous name). > >> > >> But when we're asking for 'annotate' for a file in some old revision > >> (under old name), it won't be the same revision where it had been > >> renamed, 99% of the time. > > > > It doesn't matter. The above-mentioned bzr command will show the > > telltale "RM old => new" status. Keep in mind that "-r REVISION" in > > bzr means "since REVISION till the current head". > > Ah, you mean some code will need to search through the changes up until > revision to find all renames anyway? Yes, similar to what vc-git/hg-file-name-changes do. > I suppose that could be implemented in vc-bzr-annotate (independent > of this changeset). Yes, most probably. ^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2023-12-16 7:21 UTC | newest] Thread overview: 18+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-06-09 9:54 bug#55871: 27.1; vc-git.el log view 'a', 'f', 'd' do not work when following renames Nicolás Ojeda Bär [not found] ` <handler.55871.B.16547851264967.ack@debbugs.gnu.org> 2022-06-10 17:31 ` bug#55871: Acknowledgement (27.1; vc-git.el log view 'a', 'f', 'd' do not work when following renames) Nicolás Ojeda Bär 2022-08-18 2:10 ` Dmitry Gutov 2022-09-06 10:56 ` bug#55871: 27.1; vc-git.el log view 'a', 'f', 'd' do not work when following renames Lars Ingebrigtsen 2022-09-06 12:12 ` Nicolás Ojeda Bär 2022-09-06 12:13 ` Lars Ingebrigtsen 2022-12-03 2:02 ` Dmitry Gutov 2022-12-11 23:02 ` bug#55871: Acknowledgement (27.1; vc-git.el log view 'a', 'f', 'd' do not work when following renames) Dmitry Gutov 2022-12-12 16:44 ` Nicolás Ojeda Bär 2022-12-13 1:23 ` Dmitry Gutov 2023-12-14 0:52 ` Dmitry Gutov 2023-12-14 1:23 ` Dmitry Gutov 2023-12-15 2:01 ` Dmitry Gutov 2023-12-15 13:05 ` Eli Zaretskii 2023-12-15 14:39 ` Dmitry Gutov 2023-12-15 15:10 ` Eli Zaretskii 2023-12-15 20:45 ` Dmitry Gutov 2023-12-16 7:21 ` Eli Zaretskii
Code repositories for project(s) associated with this public inbox https://git.savannah.gnu.org/cgit/emacs.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).