* bug#68183: 28.3; vc-dir fails when I have a certain branch checked out @ 2023-12-31 18:59 Tom Tromey 2023-12-31 19:34 ` Eli Zaretskii 0 siblings, 1 reply; 29+ messages in thread From: Tom Tromey @ 2023-12-31 18:59 UTC (permalink / raw) To: 68183 I use vc-dir frequently. When I check out one particular branch of my gdb repository, vc-dir fails with this error: vc-do-command: Failed (status 2): git --no-pager remote get-url . . The only thing that might be peculiar about this branch is that it uses another local branch as its tracking branch. If I check out other branches in this repository -- ones that track remote branches -- vc-dir works again. In GNU Emacs 28.3 (build 1, x86_64-redhat-linux-gnu, GTK+ Version 3.24.38, cairo version 1.17.8) of 2023-09-23 built on fd97b702fbea4aa3b70f5e90b3f3f165 Windowing system distributor 'The X.Org Foundation', version 11.0.12201009 System Description: Fedora Linux 38 (Workstation Edition) Configured using: 'configure --build=x86_64-redhat-linux-gnu --host=x86_64-redhat-linux-gnu --program-prefix= --disable-dependency-tracking --prefix=/usr --exec-prefix=/usr --bindir=/usr/bin --sbindir=/usr/sbin --sysconfdir=/etc --datadir=/usr/share --includedir=/usr/include --libdir=/usr/lib64 --libexecdir=/usr/libexec --localstatedir=/var --runstatedir=/run --sharedstatedir=/var/lib --mandir=/usr/share/man --infodir=/usr/share/info --with-dbus --with-gif --with-jpeg --with-png --with-rsvg --with-tiff --with-xpm --with-x-toolkit=gtk3 --with-gpm=no --with-xwidgets --with-modules --with-harfbuzz --with-cairo --with-json --with-native-compilation build_alias=x86_64-redhat-linux-gnu host_alias=x86_64-redhat-linux-gnu CC=gcc 'CFLAGS=-DMAIL_USE_LOCKF -O2 -flto=auto -ffat-lto-objects -fexceptions -g -grecord-gcc-switches -pipe -Wall -Werror=format-security -Wp,-U_FORTIFY_SOURCE,-D_FORTIFY_SOURCE=3 -Wp,-D_GLIBCXX_ASSERTIONS -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -fstack-protector-strong -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -m64 -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection -fno-omit-frame-pointer -mno-omit-leaf-frame-pointer ' LDFLAGS=-Wl,-z,relro PKG_CONFIG_PATH=:/usr/lib64/pkgconfig:/usr/share/pkgconfig' Configured features: ACL CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GSETTINGS HARFBUZZ JPEG JSON LIBOTF LIBSELINUX LIBSYSTEMD LIBXML2 M17N_FLT MODULES NATIVE_COMP NOTIFY INOTIFY PDUMPER PNG RSVG SECCOMP SOUND THREADS TIFF TOOLKIT_SCROLL_BARS X11 XDBE XIM XPM XWIDGETS GTK3 ZLIB Important settings: value of $LANG: en_US.UTF-8 value of $XMODIFIERS: @im=ibus locale-coding-system: utf-8-unix Major mode: Compilation Minor modes in effect: shell-dirtrack-mode: t which-function-mode: t erc-services-mode: t erc-networks-mode: t savehist-mode: t tooltip-mode: t global-eldoc-mode: t show-paren-mode: t electric-indent-mode: t mouse-wheel-mode: t file-name-shadow-mode: t global-font-lock-mode: t font-lock-mode: t auto-composition-mode: t auto-encryption-mode: t auto-compression-mode: t buffer-read-only: t column-number-mode: t line-number-mode: t indent-tabs-mode: t transient-mark-mode: t Load-path shadows: /home/tromey/.emacs.d/elpa/bubbles-0.5/bubbles hides /usr/share/emacs/28.3/lisp/play/bubbles /home/tromey/.emacs.d/elpa/dictionary-1.10/dictionary hides /usr/share/emacs/28.3/lisp/net/dictionary Features: (shadow emacsbug markdown-mode tcl m4-mode gud novice pcmpl-unix pcmpl-gnu two-column url-http url-gw url-auth sh-script smie executable dired-aux rng-xsd xsd-regexp rng-cmpct rng-nxml rng-valid rng-loc rng-uri rng-parse nxml-parse rng-match rng-dt rng-util rng-pttrn nxml-ns nxml-mode nxml-outln nxml-rap sgml-mode facemenu nxml-util nxml-enc xmltok autoconf autoconf-mode gnus-html help-fns radix-tree url-cache org-bullets org-element avl-tree ol-eww eww xdg url-queue mm-url ol-rmail ol-mhe ol-irc ol-info ol-gnus nnselect gnus-search eieio-opt speedbar ezimage dframe ol-docview doc-view image-mode exif ol-bibtex ol-bbdb ol-w3m ol-doi org-link-doi org ob ob-tangle ob-ref ob-lob ob-table ob-exp org-macro org-footnote org-src ob-comint org-pcomplete org-list org-faces org-entities noutline outline org-version ob-emacs-lisp ob-core ob-eval org-table oc-basic bibtex ol org-keys oc org-compat org-macs org-loaddefs find-func flow-fill python tramp-sh term/xterm xterm face-remap goto-addr log-edit vc-annotate mule-util jka-compr find-dired texinfo texinfo-loaddefs find-file make-mode log-view pcvs-util vc-mtn vc-hg vc-bzr vc-src vc-sccs vc-svn vc-cvs vc-rcs copyright pulse ffap scheme mailalias dabbrev supercite regi mail-hist ggtags hippie-exp etags fileloop generator xref project bug-reference vc-git cc-mode cc-fonts cc-guess cc-menus cc-cmds smerge-mode diff diff-mode shr-color mm-archive sort smiley gnus-cite mail-extr qp gnus-async gnus-bcklg gnus-ml disp-table misearch multi-isearch gnus-topic nndraft nnmh nnfolder utf-7 gnutls network-stream nsm gnus-agent gnus-srvr gnus-score score-mode nnvirtual gnus-msg gnus-art mm-uu mml2015 mm-view mml-smime smime dig nntp gnus-cache gnus-sum shr kinsoku svg dom gnus-group gnus-undo smtpmail sendmail gnus-start gnus-dbus dbus gnus-cloud nnimap nnmail mail-source utf7 netrc nnoo gnus-spec gnus-int gnus-range message rmc puny dired dired-loaddefs rfc822 mml mml-sec epa derived epg rfc6068 epg-config mm-decode mm-bodies mm-encode mail-parse rfc2231 mailabbrev gmm-utils mailheader gnus-win gnus nnheader gnus-util rmail rmail-loaddefs rfc2047 rfc2045 ietf-drums mail-utils mm-util mail-prsvr add-log flyspell ispell diminish projectile ibuf-macs pcase edmacro kmacro grep compile text-property-search ibuf-ext ibuffer ibuffer-loaddefs dash appt diary-lib diary-loaddefs cal-menu calendar cal-loaddefs tramp tramp-loaddefs trampver tramp-integration files-x tramp-compat shell pcomplete parse-time ls-lisp which-func imenu minimap autorevert filenotify cus-load erc-track erc-match erc-services erc-networks erc-hl-nicks easy-mmode color erc-button erc-fill erc-stamp wid-edit erc-goodies erc erc-backend iso8601 time-date thingatpt pp format-spec erc-loaddefs comp comp-cstr rx cl-extra help-mode warnings advice vc-dir ewoc vc vc-dispatcher cc-styles cc-align cc-engine cc-vars cc-defs ange-ftp comint ansi-color ring server savehist clang-rename clang-include-fixer let-alist clang-format xml finder-inf gdb-shell-autoloads lisppaste-autoloads pydoc-info-autoloads info-look info cl weblogger-autoloads package browse-url url url-proxy url-privacy url-expand url-methods url-history url-cookie url-domsuf url-util mailcap 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 iso-transl tooltip eldoc paren electric uniquify ediff-hook vc-hooks lisp-float-type elisp-mode mwheel term/x-win x-win term/common-win x-dnd tool-bar dnd fontset image regexp-opt fringe tabulated-list replace newcomment text-mode lisp-mode prog-mode register page tab-bar menu-bar rfn-eshadow isearch easymenu timer select scroll-bar mouse jit-lock font-lock syntax 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 emoji-zwj charscript charprop case-table epa-hook jka-cmpr-hook help simple abbrev obarray cl-preloaded nadvice button loaddefs faces cus-face macroexp files window text-properties overlay sha1 md5 base64 format env code-pages mule custom widget hashtable-print-readable backquote threads xwidget-internal dbusbind inotify dynamic-setting system-font-setting font-render-setting cairo move-toolbar gtk x-toolkit x multi-tty make-network-process native-compile emacs) Memory information: ((conses 16 2511390 280777) (symbols 48 68379 41) (strings 32 200925 47137) (string-bytes 1 9410331) (vectors 16 109364) (vector-slots 8 3054321 332806) (floats 8 532 632) (intervals 56 469509 4701) (buffers 992 1022)) ^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#68183: 28.3; vc-dir fails when I have a certain branch checked out 2023-12-31 18:59 bug#68183: 28.3; vc-dir fails when I have a certain branch checked out Tom Tromey @ 2023-12-31 19:34 ` Eli Zaretskii 2023-12-31 20:14 ` Tom Tromey 0 siblings, 1 reply; 29+ messages in thread From: Eli Zaretskii @ 2023-12-31 19:34 UTC (permalink / raw) To: Tom Tromey; +Cc: 68183 > From: Tom Tromey <tom@tromey.com> > Date: Sun, 31 Dec 2023 11:59:11 -0700 > > > I use vc-dir frequently. > > When I check out one particular branch of my gdb repository, vc-dir > fails with this error: > > vc-do-command: Failed (status 2): git --no-pager remote get-url . . > > The only thing that might be peculiar about this branch is that it uses > another local branch as its tracking branch. > > If I check out other branches in this repository -- ones that track > remote branches -- vc-dir works again. Thanks, but I think we'd appreciate a reproducible recipe for this: how can one create a Git repository which can be used to reproduce this issue? ^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#68183: 28.3; vc-dir fails when I have a certain branch checked out 2023-12-31 19:34 ` Eli Zaretskii @ 2023-12-31 20:14 ` Tom Tromey 2024-01-03 9:46 ` Kévin Le Gouguec 0 siblings, 1 reply; 29+ messages in thread From: Tom Tromey @ 2023-12-31 20:14 UTC (permalink / raw) To: Eli Zaretskii; +Cc: Tom Tromey, 68183 Eli> Thanks, but I think we'd appreciate a reproducible recipe for this: Eli> how can one create a Git repository which can be used to reproduce Eli> this issue? This worked for me: $ cd ~/Emacs/trunk # This is my Emacs git repository $ git checkout --track -b vc-dir-bug master branch 'vc-dir-bug' set up to track 'master'. Switched to a new branch 'vc-dir-bug' Now invoke vc-dir on that directory. Tom ^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#68183: 28.3; vc-dir fails when I have a certain branch checked out 2023-12-31 20:14 ` Tom Tromey @ 2024-01-03 9:46 ` Kévin Le Gouguec 2024-02-12 8:08 ` Kévin Le Gouguec 0 siblings, 1 reply; 29+ messages in thread From: Kévin Le Gouguec @ 2024-01-03 9:46 UTC (permalink / raw) To: Tom Tromey; +Cc: Eli Zaretskii, 68183 Tom Tromey <tom@tromey.com> writes: > Eli> Thanks, but I think we'd appreciate a reproducible recipe for this: > Eli> how can one create a Git repository which can be used to reproduce > Eli> this issue? > > This worked for me: > > $ cd ~/Emacs/trunk > # This is my Emacs git repository > $ git checkout --track -b vc-dir-bug master > branch 'vc-dir-bug' set up to track 'master'. > Switched to a new branch 'vc-dir-bug' > > > Now invoke vc-dir on that directory. I can reproduce; IIUC the salient point is setting start-point to a local revision when calling git checkout, by opposition to e.g. origin/master. Continuing off of your recipe: $ git branch --set-upstream-to origin/master Then M-x vc-dir works again. IIUC, to display Remote : https://git.savannah.gnu.org/git/emacs.git vc-git-dir-extra-headers runs 1. git config branch.vc-dir-bug.remote ⇒ "." 2. (vc-git-repository-url "[… EMACS DIR …]" ".") 1. git config remote...url ⇒ error git-config(1) says that branch.<name>.remote is "." when <name> is tracking a local branch, whereas branch.<name>.merge points to the local branch 'git pull' will resync with. Wonder what TRT would be for the purposes of vc-dir? (1) Drop the "Remote" header: the current branch is not sync'd with a remote branch, after all. (2) Print "Remote: https://git.savannah.gnu.org/git/emacs.git" by making vc-git-repository-url fall back to remote.origin.url when remote-name is ".". (3) Print "Remote: master" by making vc-git-dir-extra-headers fall back to branch.<name>.merge when .remote is ".". (4) Make vc-git-dir-extra-headers fall back to branch.<branch.<name>.merge>.remote. In our example, that would yield "Remote: https://git.savannah.gnu.org/git/emacs.git", but in general that seems unreliable, since that remote could be "." as well, and nothing prevents cycles AFAIU. IMO (3) would be the most robust, though maybe confusing (calling a local branch "remote"); (2) makes sense as well since vc-git-repository-url already falls back to remote.origin.url when remote-name is nil. (1) sounds trivially "robust" and "not too incorrect", but maybe not the most helpful. (4) is under-specified and I'm not convinced it is possible to make it generally useful. Hope I've not mis-diagnosed the problem; apologies for the noise if so. ^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#68183: 28.3; vc-dir fails when I have a certain branch checked out 2024-01-03 9:46 ` Kévin Le Gouguec @ 2024-02-12 8:08 ` Kévin Le Gouguec 2024-02-14 19:56 ` Kévin Le Gouguec 0 siblings, 1 reply; 29+ messages in thread From: Kévin Le Gouguec @ 2024-02-12 8:08 UTC (permalink / raw) To: Tom Tromey; +Cc: Dmitry Gutov, Eli Zaretskii, 68183, Juri Linkov [-- Attachment #1: Type: text/plain, Size: 2044 bytes --] Kévin Le Gouguec <kevin.legouguec@gmail.com> writes: > Tom Tromey <tom@tromey.com> writes: > >> Eli> Thanks, but I think we'd appreciate a reproducible recipe for this: >> Eli> how can one create a Git repository which can be used to reproduce >> Eli> this issue? >> >> This worked for me: >> >> $ cd ~/Emacs/trunk >> # This is my Emacs git repository >> $ git checkout --track -b vc-dir-bug master >> branch 'vc-dir-bug' set up to track 'master'. >> Switched to a new branch 'vc-dir-bug' >> >> >> Now invoke vc-dir on that directory. > […] > git-config(1) says that branch.<name>.remote is "." when <name> is > tracking a local branch, whereas branch.<name>.merge points to the local > branch 'git pull' will resync with. Wonder what TRT would be for the > purposes of vc-dir? Here's a patch. tl;dr (1) When branch.<name>.remote is ".", display… > Remote : none (tracking local branch) … instead of raising an error. (2) When branch.<name>.merge is set to BRANCH, display… > Tracking : BRANCH (3) Add tests, because why not. CC'ing Dmitry and Juri, whom my brain associates with vc-git maintenance (rightly or wrongly; apologies for the noise if the latter). I might be slow to respond for the next couple of weeks, so please feel free to adapt or dismiss the patch as convenient. FWIW, off the top of my head, * the test should probably have a (skip-unless (have-git-or-something)), * maybe "none (tracking local branch)" is not informative and we should ditch it, * maybe we should fall back to "origin", like vc-git-repository-url does, * rushed the ChangeLog entry; vc-git-test--run should also be declared as a "new helper" (and maybe I should spell out that I used it to not have to depend on vc-git-- internal functions), * maybe the new header deserves a NEWS entry. I can tackle any of the above (and any other feedback), though again, if someone has a vision and I don't answer in a timely manner, don't wait on me. [-- Attachment #2: 0001-Fix-vc-dir-when-remote-Git-branch-is-local.patch --] [-- Type: text/x-patch, Size: 6680 bytes --] From ba9b1f6f340a551acab0014c69f05f38191bd345 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Le=20Gouguec?= <kevin.legouguec@gmail.com> Date: Mon, 12 Feb 2024 08:29:19 +0100 Subject: [PATCH] Fix vc-dir when "remote" Git branch is local MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit While in there, add that "tracking" branch to the vc-dir buffer. For bug#68183. * lisp/vc/vc-git.el (vc-git-dir-extra-headers): Reduce boilerplate with new function 'vc-git--out-ok'; skip calling vc-git-repository-url when REMOTE is "."; display tracking branch; prefer "none (<details…>)" to "not (<details…>)" since that reads more grammatically correct. (vc-git--out-ok): Add documentation. (vc-git--out-str): New function to easily get the output from a Git command. * test/lisp/vc/vc-git-tests.el (vc-git-test--with-repo): New helper. (vc-git-test--run, vc-git-test-dir-track-local-branch): Check that vc-dir does not crash. --- lisp/vc/vc-git.el | 44 ++++++++++++++++++++++++------------ test/lisp/vc/vc-git-tests.el | 39 ++++++++++++++++++++++++++++++++ 2 files changed, 69 insertions(+), 14 deletions(-) diff --git a/lisp/vc/vc-git.el b/lisp/vc/vc-git.el index 456417e566e..bf3032e5537 100644 --- a/lisp/vc/vc-git.el +++ b/lisp/vc/vc-git.el @@ -817,27 +817,29 @@ vc-git--cmds-in-progress cmds)) (defun vc-git-dir-extra-headers (dir) - (let ((str (with-output-to-string - (with-current-buffer standard-output - (vc-git--out-ok "symbolic-ref" "HEAD")))) + (let ((str (vc-git--out-str "symbolic-ref" "HEAD")) (stash-list (vc-git-stash-list)) (default-directory dir) (in-progress (vc-git--cmds-in-progress)) - branch remote remote-url stash-button stash-string) + branch remote-url stash-button stash-string upstream-branch) (if (string-match "^\\(refs/heads/\\)?\\(.+\\)$" str) (progn (setq branch (match-string 2 str)) - (setq remote - (with-output-to-string - (with-current-buffer standard-output - (vc-git--out-ok "config" - (concat "branch." branch ".remote"))))) - (when (string-match "\\([^\n]+\\)" remote) - (setq remote (match-string 1 remote))) - (when (> (length remote) 0) - (setq remote-url (vc-git-repository-url dir remote)))) - (setq branch "not (detached HEAD)")) + (let ((remote (vc-git--out-str + "config" (concat "branch." branch ".remote"))) + (merge (vc-git--out-str + "config" (concat "branch." branch ".merge")))) + (when (string-match "\\([^\n]+\\)" remote) + (setq remote (match-string 1 remote))) + (pcase remote + ("." + (setq remote-url "none (tracking local branch)")) + ((pred (not string-empty-p)) + (setq remote-url (vc-git-repository-url dir remote)))) + (when (string-match "^\\(refs/heads/\\)?\\(.+\\)$" merge) + (setq upstream-branch (match-string 2 merge))))) + (setq branch "none (detached HEAD)")) (when stash-list (let* ((len (length stash-list)) (limit @@ -896,6 +898,11 @@ vc-git-dir-extra-headers (propertize "Remote : " 'face 'vc-dir-header) (propertize remote-url 'face 'vc-dir-header-value))) + (when upstream-branch + (concat + "\n" + (propertize "Tracking : " 'face 'vc-dir-header) + (propertize upstream-branch 'face 'vc-dir-header-value))) ;; For now just a heading, key bindings can be added later for various bisect actions (when (memq 'bisect in-progress) (propertize "\nBisect : in progress" 'face 'vc-dir-status-warning)) @@ -2219,8 +2226,17 @@ vc-git--call (apply #'process-file vc-git-program nil buffer nil "--no-pager" command args))) (defun vc-git--out-ok (command &rest args) + "Run `git COMMAND ARGS...' and insert standard output in current buffer. +Return whether the process exited with status zero." (zerop (apply #'vc-git--call '(t nil) command args))) +(defun vc-git--out-str (command &rest args) + "Run `git COMMAND ARGS...' and return standard output. +The exit status is ignored." + (with-output-to-string + (with-current-buffer standard-output + (apply #'vc-git--out-ok command args)))) + (defun vc-git--run-command-string (file &rest args) "Run a git command on FILE and return its output as string. FILE can be nil." diff --git a/test/lisp/vc/vc-git-tests.el b/test/lisp/vc/vc-git-tests.el index c52cd9c5875..1f8abe7b9b0 100644 --- a/test/lisp/vc/vc-git-tests.el +++ b/test/lisp/vc/vc-git-tests.el @@ -24,6 +24,8 @@ ;;; Code: +(require 'ert-x) +(require 'vc) (require 'vc-git) (ert-deftest vc-git-test-program-version-general () @@ -81,4 +83,41 @@ vc-git-test-annotate-time (should-not (vc-git-annotate-time)) (should-not (vc-git-annotate-time)))) +(defmacro vc-git-test--with-repo (name &rest body) + "Initialize a repository in a temporary directory and evaluate BODY. + +The current directory will be set to the top of that repository; NAME +will be bound to that directory's file name. Once BODY exits, the +directory will be deleted." + (declare (indent 1)) + `(ert-with-temp-directory ,name + (let ((default-directory ,name)) + (vc-create-repo 'Git) + ,@body))) + +(defun vc-git-test--run (&rest args) + "Run git ARGS…, check for non-zero status, and return output." + (with-temp-buffer + (apply 'vc-git-command t 0 nil args) + (buffer-string))) + +(ert-deftest vc-git-test-dir-track-local-branch () + "Test that `vc-dir' works when tracking local branches. See bug#68183." + (vc-git-test--with-repo repo + ;; Create an initial commit to get a branch started. + (write-region "hello" nil "README") + (vc-git-test--run "add" "README") + (vc-git-test--run "commit" "-mFirst") + ;; Get current branch name lazily, to remain agnostic of + ;; init.defaultbranch. + (let ((upstream-branch + (string-trim (vc-git-test--run "branch" "--show-current")))) + (vc-git-test--run "checkout" "--track" "-b" "hack" upstream-branch) + (vc-dir default-directory) + (pcase-dolist (`(,header ,value) + `(("Branch" "hack") + ("Tracking" ,upstream-branch))) + (goto-char (point-min)) + (re-search-forward (format "^%s *: %s$" header value)))))) + ;;; vc-git-tests.el ends here -- 2.43.0 ^ permalink raw reply related [flat|nested] 29+ messages in thread
* bug#68183: 28.3; vc-dir fails when I have a certain branch checked out 2024-02-12 8:08 ` Kévin Le Gouguec @ 2024-02-14 19:56 ` Kévin Le Gouguec 2024-03-13 20:03 ` Kévin Le Gouguec 2024-03-15 2:57 ` Dmitry Gutov 0 siblings, 2 replies; 29+ messages in thread From: Kévin Le Gouguec @ 2024-02-14 19:56 UTC (permalink / raw) To: Tom Tromey; +Cc: Dmitry Gutov, Eli Zaretskii, 68183, Juri Linkov [-- Attachment #1: Type: text/plain, Size: 2196 bytes --] Kévin Le Gouguec <kevin.legouguec@gmail.com> writes: > Kévin Le Gouguec <kevin.legouguec@gmail.com> writes: > >> Tom Tromey <tom@tromey.com> writes: >> >>> Eli> Thanks, but I think we'd appreciate a reproducible recipe for this: >>> Eli> how can one create a Git repository which can be used to reproduce >>> Eli> this issue? >>> >>> This worked for me: >>> >>> $ cd ~/Emacs/trunk >>> # This is my Emacs git repository >>> $ git checkout --track -b vc-dir-bug master >>> branch 'vc-dir-bug' set up to track 'master'. >>> Switched to a new branch 'vc-dir-bug' >>> >>> >>> Now invoke vc-dir on that directory. >> […] > Here's a patch. […] And here's another revision, addressing most of the points below. WDY'allT? > * the test should probably have a (skip-unless (have-git-or-something)), Done. > * maybe "none (tracking local branch)" is not informative and we should > ditch it, > * maybe we should fall back to "origin", like vc-git-repository-url > does, FWIW, the current patch will show Branch : vc-dir-tracking-branch Tracking : origin/master Remote : https://git.savannah.gnu.org/git/emacs.git for my checkout of this work-in-progress patch, and Branch : vc-dir-bug Tracking : master Remote : none (tracking local branch) for a checkout made following Tom's recipe, and Branch : trunk for a fresh 'git init' with just a default branch. OT1H "none (tracking local branch)" is redundant with "Tracking" not being prefixed with "origin"; OTOH * stripping "Remote" altogether might confuse users - at least "tracking local branch" hints at what's going on, * Falling back to origin's URL might also cause confusion: users might then expect 'vc-pull' to fetch changes from that URL, which is not the case. So all in all I think the above is reasonably useful. > * rushed the ChangeLog entry; vc-git-test--run should also be declared > as a "new helper" (and maybe I should spell out that I used it to not > have to depend on vc-git-- internal functions), Done. > * maybe the new header deserves a NEWS entry. Maybe? [-- Attachment #2: 0001-Fix-vc-dir-when-remote-Git-branch-is-local.patch --] [-- Type: text/x-diff, Size: 6732 bytes --] From ccca29fe2cce9b8c3477fcd31c8b37b1d8a57e94 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Le=20Gouguec?= <kevin.legouguec@gmail.com> Date: Mon, 12 Feb 2024 08:29:19 +0100 Subject: [PATCH] Fix vc-dir when "remote" Git branch is local MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit While in there, add that "tracking" branch to the vc-dir buffer. For bug#68183. * lisp/vc/vc-git.el (vc-git-dir-extra-headers): Reduce boilerplate with new function 'vc-git--out-ok'; stop calling vc-git-repository-url when REMOTE is "." to avoid throwing an error; display tracking branch; prefer "none (<details…>)" to "not (<details…>)" since that reads more grammatically correct. (vc-git--out-ok): Add documentation. (vc-git--out-str): New function to easily get the output from a Git command. * test/lisp/vc/vc-git-tests.el (vc-git-test--with-repo) (vc-git-test--run): New helpers, defined to steer clear of vc-git-- internal functions. (vc-git-test-dir-track-local-branch): Check that vc-dir does not crash. --- lisp/vc/vc-git.el | 46 +++++++++++++++++++++++++----------- test/lisp/vc/vc-git-tests.el | 40 +++++++++++++++++++++++++++++++ 2 files changed, 72 insertions(+), 14 deletions(-) diff --git a/lisp/vc/vc-git.el b/lisp/vc/vc-git.el index 456417e566e..b1d3dd533c6 100644 --- a/lisp/vc/vc-git.el +++ b/lisp/vc/vc-git.el @@ -817,27 +817,31 @@ vc-git--cmds-in-progress cmds)) (defun vc-git-dir-extra-headers (dir) - (let ((str (with-output-to-string - (with-current-buffer standard-output - (vc-git--out-ok "symbolic-ref" "HEAD")))) + (let ((str (vc-git--out-str "symbolic-ref" "HEAD")) (stash-list (vc-git-stash-list)) (default-directory dir) (in-progress (vc-git--cmds-in-progress)) - branch remote remote-url stash-button stash-string) + branch remote-url stash-button stash-string tracking-branch) (if (string-match "^\\(refs/heads/\\)?\\(.+\\)$" str) (progn (setq branch (match-string 2 str)) - (setq remote - (with-output-to-string - (with-current-buffer standard-output - (vc-git--out-ok "config" - (concat "branch." branch ".remote"))))) - (when (string-match "\\([^\n]+\\)" remote) - (setq remote (match-string 1 remote))) - (when (> (length remote) 0) - (setq remote-url (vc-git-repository-url dir remote)))) - (setq branch "not (detached HEAD)")) + (let ((remote (vc-git--out-str + "config" (concat "branch." branch ".remote"))) + (merge (vc-git--out-str + "config" (concat "branch." branch ".merge")))) + (when (string-match "\\([^\n]+\\)" remote) + (setq remote (match-string 1 remote))) + (when (string-match "^\\(refs/heads/\\)?\\(.+\\)$" merge) + (setq tracking-branch (match-string 2 merge))) + (pcase remote + ("." + (setq remote-url "none (tracking local branch)")) + ((pred (not string-empty-p)) + (setq + remote-url (vc-git-repository-url dir remote) + tracking-branch (concat remote "/" tracking-branch)))))) + (setq branch "none (detached HEAD)")) (when stash-list (let* ((len (length stash-list)) (limit @@ -890,6 +894,11 @@ vc-git-dir-extra-headers (propertize "Branch : " 'face 'vc-dir-header) (propertize branch 'face 'vc-dir-header-value) + (when tracking-branch + (concat + "\n" + (propertize "Tracking : " 'face 'vc-dir-header) + (propertize tracking-branch 'face 'vc-dir-header-value))) (when remote-url (concat "\n" @@ -2219,8 +2228,17 @@ vc-git--call (apply #'process-file vc-git-program nil buffer nil "--no-pager" command args))) (defun vc-git--out-ok (command &rest args) + "Run `git COMMAND ARGS...' and insert standard output in current buffer. +Return whether the process exited with status zero." (zerop (apply #'vc-git--call '(t nil) command args))) +(defun vc-git--out-str (command &rest args) + "Run `git COMMAND ARGS...' and return standard output. +The exit status is ignored." + (with-output-to-string + (with-current-buffer standard-output + (apply #'vc-git--out-ok command args)))) + (defun vc-git--run-command-string (file &rest args) "Run a git command on FILE and return its output as string. FILE can be nil." diff --git a/test/lisp/vc/vc-git-tests.el b/test/lisp/vc/vc-git-tests.el index c52cd9c5875..fd3e8ccd602 100644 --- a/test/lisp/vc/vc-git-tests.el +++ b/test/lisp/vc/vc-git-tests.el @@ -24,6 +24,8 @@ ;;; Code: +(require 'ert-x) +(require 'vc) (require 'vc-git) (ert-deftest vc-git-test-program-version-general () @@ -81,4 +83,42 @@ vc-git-test-annotate-time (should-not (vc-git-annotate-time)) (should-not (vc-git-annotate-time)))) +(defmacro vc-git-test--with-repo (name &rest body) + "Initialize a repository in a temporary directory and evaluate BODY. + +The current directory will be set to the top of that repository; NAME +will be bound to that directory's file name. Once BODY exits, the +directory will be deleted." + (declare (indent 1)) + `(ert-with-temp-directory ,name + (let ((default-directory ,name)) + (vc-create-repo 'Git) + ,@body))) + +(defun vc-git-test--run (&rest args) + "Run git ARGS…, check for non-zero status, and return output." + (with-temp-buffer + (apply 'vc-git-command t 0 nil args) + (buffer-string))) + +(ert-deftest vc-git-test-dir-track-local-branch () + "Test that `vc-dir' works when tracking local branches. Bug#68183." + (skip-unless (executable-find vc-git-program)) + (vc-git-test--with-repo repo + ;; Create an initial commit to get a branch started. + (write-region "hello" nil "README") + (vc-git-test--run "add" "README") + (vc-git-test--run "commit" "-mFirst") + ;; Get current branch name lazily, to remain agnostic of + ;; init.defaultbranch. + (let ((upstream-branch + (string-trim (vc-git-test--run "branch" "--show-current")))) + (vc-git-test--run "checkout" "--track" "-b" "hack" upstream-branch) + (vc-dir default-directory) + (pcase-dolist (`(,header ,value) + `(("Branch" "hack") + ("Tracking" ,upstream-branch))) + (goto-char (point-min)) + (re-search-forward (format "^%s *: %s$" header value)))))) + ;;; vc-git-tests.el ends here -- 2.39.2 ^ permalink raw reply related [flat|nested] 29+ messages in thread
* bug#68183: 28.3; vc-dir fails when I have a certain branch checked out 2024-02-14 19:56 ` Kévin Le Gouguec @ 2024-03-13 20:03 ` Kévin Le Gouguec 2024-03-15 2:57 ` Dmitry Gutov 1 sibling, 0 replies; 29+ messages in thread From: Kévin Le Gouguec @ 2024-03-13 20:03 UTC (permalink / raw) To: Tom Tromey; +Cc: Dmitry Gutov, Eli Zaretskii, 68183, Juri Linkov Hello folks, Kévin Le Gouguec <kevin.legouguec@gmail.com> writes: > And here's another revision, addressing most of the points below. > WDY'allT? > […] >> * maybe "none (tracking local branch)" is not informative and we should >> ditch it, >> * maybe we should fall back to "origin", like vc-git-repository-url >> does, > > FWIW, the current patch will show > > Branch : vc-dir-tracking-branch > Tracking : origin/master > Remote : https://git.savannah.gnu.org/git/emacs.git > > for my checkout of this work-in-progress patch, and > > Branch : vc-dir-bug > Tracking : master > Remote : none (tracking local branch) > > for a checkout made following Tom's recipe, and > > Branch : trunk > > for a fresh 'git init' with just a default branch. Wondering if there's anything I can do to help with review, or if I should install this. OT1H I don't want to be pushy, OTOH I should be in the area to answer for any collateral damage. ^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#68183: 28.3; vc-dir fails when I have a certain branch checked out 2024-02-14 19:56 ` Kévin Le Gouguec 2024-03-13 20:03 ` Kévin Le Gouguec @ 2024-03-15 2:57 ` Dmitry Gutov 2024-03-16 17:56 ` Kévin Le Gouguec 1 sibling, 1 reply; 29+ messages in thread From: Dmitry Gutov @ 2024-03-15 2:57 UTC (permalink / raw) To: Kévin Le Gouguec, Tom Tromey; +Cc: Eli Zaretskii, 68183, Juri Linkov Hi! Sorry about the late reply. It seems like you've done a fair amount of testing, both manual and automated - thanks, more tests are welcome. On 14/02/2024 21:56, Kévin Le Gouguec wrote: > And here's another revision, addressing most of the points below. > WDY'allT? > >> * the test should probably have a (skip-unless (have-git-or-something)), > Done. > >> * maybe "none (tracking local branch)" is not informative and we should >> ditch it, >> * maybe we should fall back to "origin", like vc-git-repository-url >> does, > FWIW, the current patch will show > > Branch : vc-dir-tracking-branch > Tracking : origin/master > Remote :https://git.savannah.gnu.org/git/emacs.git > > for my checkout of this work-in-progress patch, and > > Branch : vc-dir-bug > Tracking : master > Remote : none (tracking local branch) > > for a checkout made following Tom's recipe, and > > Branch : trunk > > for a fresh 'git init' with just a default branch. IIUC you're adding the new "Tracking" header to the output? That seems like it should be helpful. Is there a way that we could/should optimize the display? I.e., I guess the most common case will be something like: Branch : foo-bar Tracking : origin/foo-bar which is not bad, but might be less useful than indicating that the current branch does not track anything (and so the next 'git push' should come with '-u'), or tracks a differently named branch. It might be more ergonomic to emphasize "irregular" scenarios and maybe even save on the extra line in the "common" one. Just a thought. Not something that needs to be addressed right now. And I might as well be off the mark here. > OT1H "none (tracking local branch)" is redundant with "Tracking" not > being prefixed with "origin"; OTOH > > * stripping "Remote" altogether might confuse users - at least "tracking > local branch" hints at what's going on, > > * Falling back to origin's URL might also cause confusion: users might > then expect 'vc-pull' to fetch changes from that URL, which is not the > case. That seems fine. > So all in all I think the above is reasonably useful. > >> * rushed the ChangeLog entry; vc-git-test--run should also be declared >> as a "new helper" (and maybe I should spell out that I used it to not >> have to depend on vc-git-- internal functions), > Done. > >> * maybe the new header deserves a NEWS entry. > Maybe? It wouldn't hurt. Up to you. Anyway, I think the patch is good to go. Please feel free to install it; whatever cosmetic changes we might like to add could be done later. ^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#68183: 28.3; vc-dir fails when I have a certain branch checked out 2024-03-15 2:57 ` Dmitry Gutov @ 2024-03-16 17:56 ` Kévin Le Gouguec 2024-03-17 1:06 ` Dmitry Gutov 0 siblings, 1 reply; 29+ messages in thread From: Kévin Le Gouguec @ 2024-03-16 17:56 UTC (permalink / raw) To: Dmitry Gutov; +Cc: Eli Zaretskii, Tom Tromey, 68183, Juri Linkov Dmitry Gutov <dmitry@gutov.dev> writes: > Sorry about the late reply. (Not at all, thanks for taking a look 🙏) >> FWIW, the current patch will show >> Branch : vc-dir-tracking-branch >> Tracking : origin/master >> Remote :https://git.savannah.gnu.org/git/emacs.git >> for my checkout of this work-in-progress patch, and >> Branch : vc-dir-bug >> Tracking : master >> Remote : none (tracking local branch) >> for a checkout made following Tom's recipe, and >> Branch : trunk >> for a fresh 'git init' with just a default branch. > > IIUC you're adding the new "Tracking" header to the output? That seems like it should be helpful. > > Is there a way that we could/should optimize the display? I.e., I guess the most common case will be something like: > > Branch : foo-bar > Tracking : origin/foo-bar Right, the current patch indeed shows this for a common-case clone of the Emacs repo: VC backend : Git Working dir: ~/src/emacs/master/ Branch : master Tracking : origin/master Remote : https://git.savannah.gnu.org/git/emacs.git > which is not bad, but might be less useful than indicating that the current branch does not track anything (and so the next 'git push' should come with '-u'), or tracks a differently named branch. It might be more ergonomic to emphasize "irregular" scenarios and maybe even save on the extra line in the "common" one. Good food for thought. Re. optimizing the display (which I interpret as reducing redundant information): as someone who often works with multiple remotes, seeing "Branch: FOO ; Tracking: origin/FOO" is actually useful, so I wouldn't want to remove the "tracking" bit unconditionally. OTOH it could surely be condensed to a single line, say Branch : master (tracking: origin/master) Likewise, in the local-tracking-branch case, we could go from Branch: : vc-dir-bug Tracking : master Remote : none (tracking local branch) to just Branch: : vc-dir-bug (tracking: local master) Re. emphasizing irregular scenarios, specifically those where 'git push' will require '-u': I like the idea, but I am wary of us getting lost in the weeds second-guessing Git's intentions. E.g. even when branch.foo.merge and branch.foo.remote are unset, 'git push' can still be called without '-u' if push.default is 'current' and remote.pushDefault is set (whereas 'git pull' will fail). > Just a thought. Not something that needs to be addressed right now. And I might as well be off the mark here. I agree it's worth thinking about. The Right Solution™ would probably come with user options to let users fine-tune which details they care about? It would be interesting to survey Git UIs to see how they approach this (FWIW I am partial to Magit's presentation, but I have not studied how it handles all the corner cases we considered). >>> * maybe the new header deserves a NEWS entry. >> Maybe? > > It wouldn't hurt. Up to you. > > Anyway, I think the patch is good to go. Please feel free to install it; whatever cosmetic changes we might like to add could be done later. ACK. I might go a head and install then (after polishing the changelog, i.e. re-filling and scrubbing Unicode ellipses) in the spirit of getting the original issue fixed; perhaps worth holding off on the NEWS entry until we decide how exactly things should look. ^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#68183: 28.3; vc-dir fails when I have a certain branch checked out 2024-03-16 17:56 ` Kévin Le Gouguec @ 2024-03-17 1:06 ` Dmitry Gutov 2024-03-17 18:09 ` Kévin Le Gouguec 0 siblings, 1 reply; 29+ messages in thread From: Dmitry Gutov @ 2024-03-17 1:06 UTC (permalink / raw) To: Kévin Le Gouguec; +Cc: Eli Zaretskii, Tom Tromey, 68183, Juri Linkov On 16/03/2024 19:56, Kévin Le Gouguec wrote: >> IIUC you're adding the new "Tracking" header to the output? That seems like it should be helpful. >> >> Is there a way that we could/should optimize the display? I.e., I guess the most common case will be something like: >> >> Branch : foo-bar >> Tracking : origin/foo-bar > > Right, the current patch indeed shows this for a common-case clone of > the Emacs repo: > > VC backend : Git > Working dir: ~/src/emacs/master/ > Branch : master > Tracking : origin/master > Remote : https://git.savannah.gnu.org/git/emacs.git > >> which is not bad, but might be less useful than indicating that the current branch does not track anything (and so the next 'git push' should come with '-u'), or tracks a differently named branch. It might be more ergonomic to emphasize "irregular" scenarios and maybe even save on the extra line in the "common" one. > > Good food for thought. > > Re. optimizing the display (which I interpret as reducing redundant > information): as someone who often works with multiple remotes, seeing > "Branch: FOO ; Tracking: origin/FOO" is actually useful, so I wouldn't > want to remove the "tracking" bit unconditionally. OTOH it could surely > be condensed to a single line, say > > Branch : master (tracking: origin/master) Yeah, something like this could work. I was imagining pseudo-graphical Branch : master -> origin/master , but it's good to have words. Maybe drop the last colon... > Likewise, in the local-tracking-branch case, we could go from > > Branch: : vc-dir-bug > Tracking : master > Remote : none (tracking local branch) > > to just > > Branch: : vc-dir-bug (tracking: local master) I would say that the local-tracking scenario is a minority, and so it's not as important to optimize, but the above makes sense. But the word "local" is very close to "master", while the latter is a special string which should probably somehow stand out. So the "unoptimized" version might still have its advantages: Branch: : vc-dir-bug (tracking master) Remote : none (local branch) or Branch: : vc-dir-bug (-> master) Remote : none (local branch) or Branch: : vc-dir-bug (tracking -> master) Remote : none (local branch) > Re. emphasizing irregular scenarios, specifically those where 'git push' > will require '-u': I like the idea, but I am wary of us getting lost in > the weeds second-guessing Git's intentions. E.g. even when > branch.foo.merge and branch.foo.remote are unset, 'git push' can still > be called without '-u' if push.default is 'current' and > remote.pushDefault is set (whereas 'git pull' will fail). Okay, if you're sure. A (no upstream) note might make sense in the above scenario too, but I don't have a strong opinion. We could also make a user option later, if the new behavior makes sense for most usage habits but not all. >> Just a thought. Not something that needs to be addressed right now. And I might as well be off the mark here. > > I agree it's worth thinking about. The Right Solution™ would probably > come with user options to let users fine-tune which details they care > about? It would be interesting to survey Git UIs to see how they > approach this (FWIW I am partial to Magit's presentation, but I have not > studied how it handles all the corner cases we considered). Magit, meaning just one line for Head: and another for Merge:? >>>> * maybe the new header deserves a NEWS entry. >>> Maybe? >> >> It wouldn't hurt. Up to you. >> >> Anyway, I think the patch is good to go. Please feel free to install it; whatever cosmetic changes we might like to add could be done later. > > ACK. I might go a head and install then (after polishing the changelog, > i.e. re-filling and scrubbing Unicode ellipses) in the spirit of getting > the original issue fixed; perhaps worth holding off on the NEWS entry > until we decide how exactly things should look. I'm okay with that. ^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#68183: 28.3; vc-dir fails when I have a certain branch checked out 2024-03-17 1:06 ` Dmitry Gutov @ 2024-03-17 18:09 ` Kévin Le Gouguec 2024-03-18 15:26 ` Dmitry Gutov 0 siblings, 1 reply; 29+ messages in thread From: Kévin Le Gouguec @ 2024-03-17 18:09 UTC (permalink / raw) To: Dmitry Gutov; +Cc: Eli Zaretskii, Tom Tromey, 68183, Juri Linkov Dmitry Gutov <dmitry@gutov.dev> writes: >> Re. optimizing the display (which I interpret as reducing redundant >> information): as someone who often works with multiple remotes, seeing >> "Branch: FOO ; Tracking: origin/FOO" is actually useful, so I wouldn't >> want to remove the "tracking" bit unconditionally. OTOH it could surely >> be condensed to a single line, say >> Branch : master (tracking: origin/master) > > Yeah, something like this could work. I was imagining pseudo-graphical > > Branch : master -> origin/master > > , but it's good to have words. Maybe drop the last colon... Right, feels like we have some horizontal space to work with, so we can spend the extra word if it conveys more meaning than an arrow (and yeah, OTOH that last colon does not bring much). >> Likewise, in the local-tracking-branch case, we could go from >> Branch: : vc-dir-bug >> Tracking : master >> Remote : none (tracking local branch) >> to just >> Branch: : vc-dir-bug (tracking: local master) > > I would say that the local-tracking scenario is a minority, and so it's not as important to optimize, but the above makes sense. But the word "local" is very close to "master", while the latter is a special string which should probably somehow stand out. So the "unoptimized" version might still have its advantages: > > Branch: : vc-dir-bug (tracking master) > Remote : none (local branch) That looks sensible. If I were to argue in favor of keeping "local" juxtaposed to the tracking branch, I'd say that in the _absence_ of an explicit qualifier in "Branch: vc-dir-bug (tracking master)", my brain might "default" to assuming we are tracking the remote "master"; a solution to let branch names stand out might then be faces: e.g. vc-git-log-view-mode paints them with change-log-list… But I don't know if it's a very powerful argument. >>> Just a thought. Not something that needs to be addressed right now. And I might as well be off the mark here. >> I agree it's worth thinking about. The Right Solution™ would probably >> come with user options to let users fine-tune which details they care >> about? It would be interesting to survey Git UIs to see how they >> approach this (FWIW I am partial to Magit's presentation, but I have not >> studied how it handles all the corner cases we considered). > > Magit, meaning just one line for Head: and another for Merge:? So, given 𝒷 ≔ current branch 𝓂 ≔ branch.𝒷.merge 𝓇 ≔ branch.𝒷.remote 𝓅 ≔ branch.𝒷.pushRemote or remote.pushDefault By default, magit-status shows: "Head:" 𝒷, or the current commit when detached "Merge:" (or "Rebase:") 𝓇/𝓂, or just 𝓂 if 𝓇 = "." "Push:" 𝓅/𝒷, appending "does not exist" if applicable (And another header related to tags I'm going to ignore for now) Of note, Magit offers an option (magit-status-headers-hook) to let users control which of these (and more) to display. One of the available headers that is disabled by default shows "Remote: ℛ <remote.ℛ.url>" where ℛ is 𝓇, or "origin" if this branch's remote is unset or ".", or "the first remote in alphabetic order" if "origin" does not exist. ( Also of note, IMO, is the branch-variable-configuration menu (the magit-branch-configure transient), which looks like this: Configure vc-dir-bug d branch.vc-dir-bug.description unset u branch.vc-dir-bug.merge refs/heads/master branch.vc-dir-bug.remote . r branch.vc-dir-bug.rebase [true|false|default:false] p branch.vc-dir-bug.pushRemote [hirondell|origin|savannah-rw|vps|remote.pushDefault:vps] Configure repository defaults R pull.rebase [true|false|default:false] P remote.pushDefault [hirondell|origin|savannah-rw|vps] b Update default branch Configure branch creation a m branch.autoSetupMerge [always|true|false|default:true] a r branch.autoSetupRebase [always|local|remote|never|default:never] __ (Not pictured: the currently active value in [foo|bar|baz] is highlighted with a face; the keybindings on the left cycle between alternatives) No idea how/where that would fit into VC, but golly is it a neat UI to work with, so I thought it deserved a shout-out. ) >>>>> * maybe the new header deserves a NEWS entry. >>>> Maybe? >>> >>> It wouldn't hurt. Up to you. >>> >>> Anyway, I think the patch is good to go. Please feel free to install it; whatever cosmetic changes we might like to add could be done later. >> ACK. I might go a head and install then (after polishing the changelog, >> i.e. re-filling and scrubbing Unicode ellipses) in the spirit of getting >> the original issue fixed; perhaps worth holding off on the NEWS entry >> until we decide how exactly things should look. > > I'm okay with that. (Pushed 📨) ^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#68183: 28.3; vc-dir fails when I have a certain branch checked out 2024-03-17 18:09 ` Kévin Le Gouguec @ 2024-03-18 15:26 ` Dmitry Gutov 2024-08-07 14:25 ` Kévin Le Gouguec 0 siblings, 1 reply; 29+ messages in thread From: Dmitry Gutov @ 2024-03-18 15:26 UTC (permalink / raw) To: Kévin Le Gouguec; +Cc: Eli Zaretskii, Tom Tromey, 68183, Juri Linkov On 17/03/2024 20:09, Kévin Le Gouguec wrote: >> I would say that the local-tracking scenario is a minority, and so it's not as important to optimize, but the above makes sense. But the word "local" is very close to "master", while the latter is a special string which should probably somehow stand out. So the "unoptimized" version might still have its advantages: >> >> Branch: : vc-dir-bug (tracking master) >> Remote : none (local branch) > > That looks sensible. If I were to argue in favor of keeping "local" > juxtaposed to the tracking branch, I'd say that in the _absence_ of an > explicit qualifier in "Branch: vc-dir-bug (tracking master)", my brain > might "default" to assuming we are tracking the remote "master"; a > solution to let branch names stand out might then be faces: > e.g. vc-git-log-view-mode paints them with change-log-list… > > But I don't know if it's a very powerful argument. I figured that when "local" is on the next line already, it's easy enough to notice. Anyway, it's up to you, I think. >>>> Just a thought. Not something that needs to be addressed right now. And I might as well be off the mark here. >>> I agree it's worth thinking about. The Right Solution™ would probably >>> come with user options to let users fine-tune which details they care >>> about? It would be interesting to survey Git UIs to see how they >>> approach this (FWIW I am partial to Magit's presentation, but I have not >>> studied how it handles all the corner cases we considered). >> >> Magit, meaning just one line for Head: and another for Merge:? > > So, given > > 𝒷 ≔ current branch > 𝓂 ≔ branch.𝒷.merge > 𝓇 ≔ branch.𝒷.remote > 𝓅 ≔ branch.𝒷.pushRemote or remote.pushDefault > > By default, magit-status shows: > > "Head:" 𝒷, or the current commit when detached > "Merge:" (or "Rebase:") 𝓇/𝓂, or just 𝓂 if 𝓇 = "." > "Push:" 𝓅/𝒷, appending "does not exist" if applicable So Magit also prints just "master" is it's a local tracking branch and something like "origin/master" when it belongs to a remote? Going with that approach, we'd seem justified to print Branch: : vc-dir-bug (tracking master) in the former case and Branch: : vc-dir-bug (tracking origin/master) in the latter. > (And another header related to tags I'm going to ignore for now) > > Of note, Magit offers an option (magit-status-headers-hook) to let users > control which of these (and more) to display. > > One of the available headers that is disabled by default shows > > "Remote: ℛ <remote.ℛ.url>" > > where ℛ is 𝓇, or "origin" if this branch's remote is unset or ".", or > "the first remote in alphabetic order" if "origin" does not exist. Meaning it prepends the remote's url with remote's name, if that line is configured to be shown. We could do that too. > ( > Also of note, IMO, is the branch-variable-configuration menu (the > magit-branch-configure transient), which looks like this: > > Configure vc-dir-bug > d branch.vc-dir-bug.description unset > u branch.vc-dir-bug.merge refs/heads/master > branch.vc-dir-bug.remote . > r branch.vc-dir-bug.rebase [true|false|default:false] > p branch.vc-dir-bug.pushRemote [hirondell|origin|savannah-rw|vps|remote.pushDefault:vps] > > Configure repository defaults > R pull.rebase [true|false|default:false] > P remote.pushDefault [hirondell|origin|savannah-rw|vps] > b Update default branch > > Configure branch creation > a m branch.autoSetupMerge [always|true|false|default:true] > a r branch.autoSetupRebase [always|local|remote|never|default:never] > __ > (Not pictured: the currently active value in [foo|bar|baz] is > highlighted with a face; the keybindings on the left cycle between > alternatives) > > No idea how/where that would fit into VC, but golly is it a neat UI to > work with, so I thought it deserved a shout-out. > ) Magit is definitely a great UI to learn Git's capabilities. >>>>>> * maybe the new header deserves a NEWS entry. >>>>> Maybe? >>>> >>>> It wouldn't hurt. Up to you. >>>> >>>> Anyway, I think the patch is good to go. Please feel free to install it; whatever cosmetic changes we might like to add could be done later. >>> ACK. I might go a head and install then (after polishing the changelog, >>> i.e. re-filling and scrubbing Unicode ellipses) in the spirit of getting >>> the original issue fixed; perhaps worth holding off on the NEWS entry >>> until we decide how exactly things should look. >> >> I'm okay with that. > > (Pushed 📨) Thanks! ^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#68183: 28.3; vc-dir fails when I have a certain branch checked out 2024-03-18 15:26 ` Dmitry Gutov @ 2024-08-07 14:25 ` Kévin Le Gouguec 2024-08-08 0:32 ` Sean Whitton 2024-08-13 1:32 ` Dmitry Gutov 0 siblings, 2 replies; 29+ messages in thread From: Kévin Le Gouguec @ 2024-08-07 14:25 UTC (permalink / raw) To: 68183; +Cc: Dmitry Gutov, Eli Zaretskii, Tom Tromey, Sean Whitton, Juri Linkov [-- Attachment #1: Type: text/plain, Size: 1204 bytes --] Heya, Have spent cycles on this on-and-off these past few months; finally have something worth discussing, I think 🤞 To recap where we stand, AFAIU: the reported vc-dir bug has been fixed (in time for the emacs-30 branch), but the changes could feel intrusive, since a new vc-dir header was added ("Tracking") that some users may not care for. I've now drafted a user option to give users more control over this new header; see patch #3 in the attached series. The first two patches are yak-shaving: patch #1 adds regression tests, patch #2 splits vc-git-dir-extra-headers into more manageable chunks (pure refactoring, no functional change intended). Also attaching a 'squashed.patch' if that helps review. About patch #2, CC'ing Sean Whitton for perspective on vc-git--cmds-in-progress: I was puzzled by the function supporting many commands (rebase, am, merge, bisect), whereas AFAICT its sole user only heeds 'bisect & 'rebase. Wondering if I've missed other in-tree uses, or if we should add headers for 'am and 'merge, "while in there". Curious what y'all think. OT1H not sure an alist is the best UX, OTOH struggled to keep option names concise otherwise. [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: 0001-Test-more-vc-dir-scenarios-with-Git-bug-68183.patch --] [-- Type: text/x-diff, Size: 5429 bytes --] From 1573015fba16f8b453e87e92e982fc633bca40d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Le=20Gouguec?= <kevin.legouguec@gmail.com> Date: Sun, 7 Jul 2024 12:16:12 +0200 Subject: [PATCH 1/3] Test more vc-dir scenarios with Git (bug#68183) * test/lisp/vc/vc-git-tests.el (vc-git-test-dir-track-local-branch): Remove in favor of new test. (vc-git-test--start-branch): New helper to get a repository going. (vc-git-test--dir-headers): New helper to get a list of headers in the current vc-dir buffer. (vc-git-test-dir-branch-headers): New test, exercising the original bug recipe plus more common scenarios. --- test/lisp/vc/vc-git-tests.el | 98 +++++++++++++++++++++++++++++------- 1 file changed, 80 insertions(+), 18 deletions(-) diff --git a/test/lisp/vc/vc-git-tests.el b/test/lisp/vc/vc-git-tests.el index f15a0f52e8c..2dbf5a8df12 100644 --- a/test/lisp/vc/vc-git-tests.el +++ b/test/lisp/vc/vc-git-tests.el @@ -26,6 +26,7 @@ (require 'ert-x) (require 'vc) +(require 'vc-dir) (require 'vc-git) (ert-deftest vc-git-test-program-version-general () @@ -108,24 +109,85 @@ vc-git-test--run (apply 'vc-git-command t 0 nil args) (buffer-string))) -(ert-deftest vc-git-test-dir-track-local-branch () - "Test that `vc-dir' works when tracking local branches. Bug#68183." +(defun vc-git-test--start-branch () + "Get a branch started in a freshly initialized repository. + +This returns the name of the current branch, so that tests can remain +agnostic of init.defaultbranch." + (write-region "hello" nil "README") + (vc-git-test--run "add" "README") + (vc-git-test--run "commit" "-mFirst") + (string-trim (vc-git-test--run "branch" "--show-current"))) + +(defun vc-git-test--dir-headers (headers) + "Return an alist of header values for the current `vc-dir' buffer. + +HEADERS should be a list of (NAME ...) strings. This function will +return a list of (NAME . VALUE) pairs, where VALUE is nil if the header +is absent." + ;; FIXME: to reproduce interactive sessions faithfully, we would need + ;; to wait for the dir-status-files process to terminate; have not + ;; found a reliable way to do this. As a workaround, kill pending + ;; processes and revert the `vc-dir' buffer. + (vc-dir-kill-dir-status-process) + (revert-buffer) + (mapcar + (lambda (header) + (let* ((pattern + (rx bol + (literal header) (* space) ": " (group (+ nonl)) + eol)) + (value (and (goto-char (point-min)) + (re-search-forward pattern nil t) + (match-string 1)))) + (cons header value))) + headers)) + +(ert-deftest vc-git-test-dir-branch-headers () + "Check that `vc-dir' shows expected branch-related headers." (skip-unless (executable-find vc-git-program)) - (vc-git-test--with-repo repo - ;; Create an initial commit to get a branch started. - (write-region "hello" nil "README") - (vc-git-test--run "add" "README") - (vc-git-test--run "commit" "-mFirst") - ;; Get current branch name lazily, to remain agnostic of - ;; init.defaultbranch. - (let ((upstream-branch - (string-trim (vc-git-test--run "branch" "--show-current")))) - (vc-git-test--run "checkout" "--track" "-b" "hack" upstream-branch) - (vc-dir default-directory) - (pcase-dolist (`(,header ,value) - `(("Branch" "hack") - ("Tracking" ,upstream-branch))) - (goto-char (point-min)) - (re-search-forward (format "^%s *: %s$" header value)))))) + ;; Create a repository that will serve as the "remote". + (vc-git-test--with-repo origin-repo + (let ((main-branch (vc-git-test--start-branch))) + ;; 'git clone' this repository and test things in this clone. + (ert-with-temp-directory clone-repo + (vc-git-test--run "clone" origin-repo clone-repo) + (vc-dir clone-repo) + (should + (equal + (vc-git-test--dir-headers + '("Branch" "Tracking" "Remote")) + `(("Branch" . ,main-branch) + ("Tracking" . ,(concat "origin/" main-branch)) + ("Remote" . ,origin-repo)))) + ;; Checkout a new branch: no tracking information. + (vc-git-test--run "checkout" "-b" "feature/foo" main-branch) + (should + (equal + (vc-git-test--dir-headers + '("Branch" "Tracking" "Remote")) + '(("Branch" . "feature/foo") + ("Tracking" . nil) + ("Remote" . nil)))) + ;; Push with '--set-upstream origin': tracking information + ;; should be updated. + (vc-git-test--run "push" "--set-upstream" "origin" "feature/foo") + (should + (equal + (vc-git-test--dir-headers + '("Branch" "Tracking" "Remote")) + `(("Branch" . "feature/foo") + ("Tracking" . "origin/feature/foo") + ("Remote" . ,origin-repo)))) + ;; Checkout a new branch tracking the _local_ main branch. + ;; Bug#68183. + (vc-git-test--run "checkout" "-b" "feature/bar" "--track" main-branch) + (should + (equal + (vc-git-test--dir-headers + '("Branch" "Tracking" "Remote")) + `(("Branch" . "feature/bar") + ("Tracking" . ,main-branch) + ("Remote" . "none (tracking local branch)")))))))) ;;; vc-git-tests.el ends here -- 2.39.2 [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #3: 0002-Split-vc-git-dir-extra-headers-into-more-manageable-.patch --] [-- Type: text/x-diff, Size: 13563 bytes --] From f4caa79c492116c3b9c6a6df7972222ee977f13c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Le=20Gouguec?= <kevin.legouguec@gmail.com> Date: Sun, 9 Jun 2024 19:41:41 +0200 Subject: [PATCH 2/3] Split vc-git-dir-extra-headers into more manageable chunks The current code requires a lot of eyeballing back-and-forth to: - check where variables are actually used, what impact changing them can have: in actuality, there are three distinct "groups" of headers we compute, each with their own independent state; - understand formatting details such as "who's in charge of the newlines". To solve both issues, split that function into smaller ones, each handling a "group" of headers. The only expected "functional" change is that, by propertizing "\nHeader: " strings, the original code sometimes applied the vc-dir-header face to the newline preceding a header; the new code applies no faces to these newlines. This change would be visible to users with themes adding an :extended background to vc-dir-header. In practice, no in-tree theme is impacted. For bug#68183. * lisp/vc/vc-git.el (vc-git-dir--branch-headers): New function to compute "Branch", "Tracking" and "Remote". (vc-git--cmds-in-progress): Rename to... (vc-git-dir--in-progress-headers): ... this, and compute headers. (vc-git-dir--stash-headers): New function to compute the "Stash" header. (vc-git-dir-extra-headers): Boil down to just setting default-directory and assembling the headers from these new helpers. (vc-git--out-match): New function to call 'git' and capture specific bits of output. --- lisp/vc/vc-git.el | 253 ++++++++++++++++++++++++---------------------- 1 file changed, 131 insertions(+), 122 deletions(-) diff --git a/lisp/vc/vc-git.el b/lisp/vc/vc-git.el index e8257c5dbd0..4d631c7e032 100644 --- a/lisp/vc/vc-git.el +++ b/lisp/vc/vc-git.el @@ -717,6 +717,63 @@ vc-git-dir-status-files :files files :update-function update-function))) +(defun vc-git-dir--branch-headers () + "Return headers for branch-related information." + (let ((branch (vc-git--out-match + '("symbolic-ref" "HEAD") + "^\\(refs/heads/\\)?\\(.+\\)$" 2)) + tracking remote-url) + (if branch + (when-let ((branch-merge + (vc-git--out-match + `("config" ,(concat "branch." branch ".merge")) + "^\\(refs/heads/\\)?\\(.+\\)$" 2)) + (branch-remote + (vc-git--out-match + `("config" ,(concat "branch." branch ".remote")) + "\\([^\n]+\\)" 1))) + (if (string= branch-remote ".") + (setq tracking branch-merge + remote-url "none (tracking local branch)") + (setq tracking (concat branch-remote "/" branch-merge) + remote-url (vc-git-repository-url + default-directory branch-remote)))) + (setq branch "none (detached HEAD)")) + (cl-flet ((fmt (key value) + (concat + (propertize (format "% -11s: " key) 'face 'vc-dir-header) + (propertize value 'face 'vc-dir-header-value)))) + (remove nil (list + (fmt "Branch" branch) + (and tracking (fmt "Tracking" tracking)) + (and remote-url (fmt "Remote" remote-url))))))) + +(defun vc-git-dir--in-progress-headers () + "Return headers for Git commands in progress in this worktree." + (let ((gitdir (vc-git--git-path)) + cmds) + ;; See contrib/completion/git-prompt.sh in git.git. + (when (or (file-directory-p + (expand-file-name "rebase-merge" gitdir)) + (file-exists-p + (expand-file-name "rebase-apply/rebasing" gitdir))) + (push 'rebase cmds)) + (when (file-exists-p + (expand-file-name "rebase-apply/applying" gitdir)) + (push 'am cmds)) + (when (file-exists-p (expand-file-name "MERGE_HEAD" gitdir)) + (push 'merge cmds)) + (when (file-exists-p (expand-file-name "BISECT_START" gitdir)) + (push 'bisect cmds)) + (cl-flet ((fmt (cmd name) + (when (memq cmd cmds) + ;; For now just a heading, key bindings can be added + ;; later for various bisect actions. + (propertize (format "% -11s: in progress" name) + 'face 'vc-dir-status-warning)))) + (remove nil (list (fmt 'bisect "Bisect") + (fmt 'rebase "Rebase")))))) + (defvar-keymap vc-git-stash-shared-map "S" #'vc-git-stash-snapshot "C" #'vc-git-stash) @@ -797,130 +854,75 @@ vc-git-stash-menu-map :help "Show the contents of the current stash")) map)) -(defun vc-git--cmds-in-progress () - "Return a list of Git commands in progress in this worktree." - (let ((gitdir (vc-git--git-path)) - cmds) - ;; See contrib/completion/git-prompt.sh in git.git. - (when (or (file-directory-p - (expand-file-name "rebase-merge" gitdir)) - (file-exists-p - (expand-file-name "rebase-apply/rebasing" gitdir))) - (push 'rebase cmds)) - (when (file-exists-p - (expand-file-name "rebase-apply/applying" gitdir)) - (push 'am cmds)) - (when (file-exists-p (expand-file-name "MERGE_HEAD" gitdir)) - (push 'merge cmds)) - (when (file-exists-p (expand-file-name "BISECT_START" gitdir)) - (push 'bisect cmds)) - cmds)) +(defun vc-git-dir--stash-headers () + "Return headers describing the current stashes." + (list + (concat + (propertize "Stash : " 'face 'vc-dir-header) + (if-let ((stash-list (vc-git-stash-list))) + (let* ((len (length stash-list)) + (limit + (if (integerp vc-git-show-stash) + (min vc-git-show-stash len) + len)) + (shown-stashes (cl-subseq stash-list 0 limit)) + (hidden-stashes (cl-subseq stash-list limit)) + (all-hideable (or (eq vc-git-show-stash t) + (<= len vc-git-show-stash)))) + (concat + ;; Button to toggle visibility. + (if all-hideable + (vc-git-make-stash-button nil limit limit) + (vc-git-make-stash-button t vc-git-show-stash len)) + ;; Stash list. + (when shown-stashes + (concat + (propertize "\n" + 'vc-git-hideable all-hideable) + (mapconcat + (lambda (x) + (propertize x + 'face 'vc-dir-header-value + 'mouse-face 'highlight + 'vc-git-hideable all-hideable + 'help-echo vc-git-stash-list-help + 'keymap vc-git-stash-map)) + shown-stashes + (propertize "\n" + 'vc-git-hideable all-hideable)))) + (when hidden-stashes + (concat + (propertize "\n" + 'invisible t + 'vc-git-hideable t) + (mapconcat + (lambda (x) + (propertize x + 'face 'vc-dir-header-value + 'mouse-face 'highlight + 'invisible t + 'vc-git-hideable t + 'help-echo vc-git-stash-list-help + 'keymap vc-git-stash-map)) + hidden-stashes + (propertize "\n" + 'invisible t + 'vc-git-hideable t)))))) + (propertize "Nothing stashed" + 'help-echo vc-git-stash-shared-help + 'keymap vc-git-stash-shared-map + 'face 'vc-dir-header-value))))) (defun vc-git-dir-extra-headers (dir) - (let ((str (vc-git--out-str "symbolic-ref" "HEAD")) - (stash-list (vc-git-stash-list)) - (default-directory dir) - (in-progress (vc-git--cmds-in-progress)) - - branch remote-url stash-button stash-string tracking-branch) - (if (string-match "^\\(refs/heads/\\)?\\(.+\\)$" str) - (progn - (setq branch (match-string 2 str)) - (let ((remote (vc-git--out-str - "config" (concat "branch." branch ".remote"))) - (merge (vc-git--out-str - "config" (concat "branch." branch ".merge")))) - (when (string-match "\\([^\n]+\\)" remote) - (setq remote (match-string 1 remote))) - (when (string-match "^\\(refs/heads/\\)?\\(.+\\)$" merge) - (setq tracking-branch (match-string 2 merge))) - (pcase remote - ("." - (setq remote-url "none (tracking local branch)")) - ((pred (not string-empty-p)) - (setq - remote-url (vc-git-repository-url dir remote) - tracking-branch (concat remote "/" tracking-branch)))))) - (setq branch "none (detached HEAD)")) - (when stash-list - (let* ((len (length stash-list)) - (limit - (if (integerp vc-git-show-stash) - (min vc-git-show-stash len) - len)) - (shown-stashes (cl-subseq stash-list 0 limit)) - (hidden-stashes (cl-subseq stash-list limit)) - (all-hideable (or (eq vc-git-show-stash t) - (<= len vc-git-show-stash)))) - (setq stash-button (if all-hideable - (vc-git-make-stash-button nil limit limit) - (vc-git-make-stash-button t vc-git-show-stash len)) - stash-string - (concat - (when shown-stashes - (concat - (propertize "\n" - 'vc-git-hideable all-hideable) - (mapconcat - (lambda (x) - (propertize x - 'face 'vc-dir-header-value - 'mouse-face 'highlight - 'vc-git-hideable all-hideable - 'help-echo vc-git-stash-list-help - 'keymap vc-git-stash-map)) - shown-stashes - (propertize "\n" - 'vc-git-hideable all-hideable)))) - (when hidden-stashes - (concat - (propertize "\n" - 'invisible t - 'vc-git-hideable t) - (mapconcat - (lambda (x) - (propertize x - 'face 'vc-dir-header-value - 'mouse-face 'highlight - 'invisible t - 'vc-git-hideable t - 'help-echo vc-git-stash-list-help - 'keymap vc-git-stash-map)) - hidden-stashes - (propertize "\n" - 'invisible t - 'vc-git-hideable t)))))))) - (concat - (propertize "Branch : " 'face 'vc-dir-header) - (propertize branch - 'face 'vc-dir-header-value) - (when tracking-branch - (concat - "\n" - (propertize "Tracking : " 'face 'vc-dir-header) - (propertize tracking-branch 'face 'vc-dir-header-value))) - (when remote-url - (concat - "\n" - (propertize "Remote : " 'face 'vc-dir-header) - (propertize remote-url - 'face 'vc-dir-header-value))) - ;; For now just a heading, key bindings can be added later for various bisect actions - (when (memq 'bisect in-progress) - (propertize "\nBisect : in progress" 'face 'vc-dir-status-warning)) - (when (memq 'rebase in-progress) - (propertize "\nRebase : in progress" 'face 'vc-dir-status-warning)) - (if stash-list - (concat - (propertize "\nStash : " 'face 'vc-dir-header) - stash-button - stash-string) - (concat - (propertize "\nStash : " 'face 'vc-dir-header) - (propertize "Nothing stashed" - 'help-echo vc-git-stash-shared-help - 'keymap vc-git-stash-shared-map - 'face 'vc-dir-header-value)))))) + (let ((default-directory dir)) + (string-join + (append + ;; Each helper returns a list of headers. Each header must be a + ;; propertized string with no final newline. + (vc-git-dir--branch-headers) + (vc-git-dir--in-progress-headers) + (vc-git-dir--stash-headers)) + "\n"))) (defun vc-git-branches () "Return the existing branches, as a list of strings. @@ -2246,6 +2248,13 @@ vc-git--out-str (with-current-buffer standard-output (apply #'vc-git--out-ok command args)))) +(defun vc-git--out-match (args regexp group) + "Run `git ARGS...' and return match for group number GROUP of REGEXP. +Return nil if the output does not match. The exit status is ignored." + (let ((out (apply #'vc-git--out-str args))) + (when (string-match regexp out) + (match-string group out)))) + (defun vc-git--run-command-string (file &rest args) "Run a git command on FILE and return its output as string. FILE can be nil." -- 2.39.2 [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #4: 0003-Let-users-choose-when-and-how-to-display-Git-trackin.patch --] [-- Type: text/x-diff, Size: 11372 bytes --] From f21546f5cae71f00a73298315f00f7693cb21d5f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Le=20Gouguec?= <kevin.legouguec@gmail.com> Date: Sun, 7 Jul 2024 19:45:49 +0200 Subject: [PATCH 3/3] Let users choose when and how to display Git tracking branch For bug#68183. * lisp/vc/vc-git.el (vc-git-dir-show-tracking): New option. (vc-git-dir--tracking): New function to format upstream branch according to the new option. (vc-git-dir--branch-headers): Use new option & new function to format upstream branch according to user preference. * test/lisp/vc/vc-git-tests.el (vc-git-test--dir-headers): Allow temporarily binding the new option. (vc-git-test-dir-branch-headers): Test a handful of option tweaks. --- lisp/vc/vc-git.el | 101 ++++++++++++++++++++++++++++++----- test/lisp/vc/vc-git-tests.el | 72 ++++++++++++++++++------- 2 files changed, 140 insertions(+), 33 deletions(-) diff --git a/lisp/vc/vc-git.el b/lisp/vc/vc-git.el index 4d631c7e032..86752bd074d 100644 --- a/lisp/vc/vc-git.el +++ b/lisp/vc/vc-git.el @@ -717,6 +717,66 @@ vc-git-dir-status-files :files files :update-function update-function))) +(defcustom vc-git-dir-show-tracking '((when . set) + (how . header)) + "Control how `vc-dir' shows the upstream branch. +The \"upstream\" branch is the one `vc-pull' fetches changes from by +default. In Git terms, when checking out branch B, the upstream branch +is defined by the configuration options branch.B.merge and +branch.B.remote. + +This option is an alist which admits the following symbol keys: + +* `when' controls whether information about the upstream branch will be + shown. The value for this key can be one of the following symbols: + - set (default) Only show the upstream branch if it is set, + as defined by the previously mentioned Git config + options. + - t If a branch is checked out (that is, HEAD is not + detached), always show something: fallback to \"none\" + if the current branch is not tracking anything. + - different Only show the upstream branch if branch.B.merge is + named differently from B. This allows hiding the + header in the common case where branch \"foo\" tracks + \"origin/foo\". + - never Never show the upstream branch. + +* `how' controls the way this information will be shown. The value can + be one of the following symbols: + - header (default) Show the branch in a dedicated header, + \"Tracking\". + - inline Append the branch to the \"Branch\" header, e.g. + Branch: foo (tracking origin/bar)" + :type 'alist + :options + '((when (radio + (const :tag "Never" never) + (const :tag "Always" t) + (const :tag "If current branch has a tracking branch" set) + (const :tag "If current & tracking branches have different names" different))) + (how (radio + (const :tag "\"Tracking\" header" header) + (const :tag "Inline in \"Branch\" header" inline)))) + :version "31.1") + +(defun vc-git-dir--tracking (branch branch-merge branch-remote) + "Return a description of BRANCH's upstream branch. +This description heeds `vc-git-dir-show-tracking'." + (cl-flet ((remote-prefix () + (if (equal branch-remote ".") + nil + (concat branch-remote "/")))) + (pcase-exhaustive (alist-get 'when vc-git-dir-show-tracking 'set) + ('set (and branch-merge + (concat (remote-prefix) branch-merge))) + ('never nil) + ('t (if branch-merge + (concat (remote-prefix) branch-merge) + "none")) + ('different (and branch-merge + (not (equal branch branch-merge)) + (concat (remote-prefix) branch-merge)))))) + (defun vc-git-dir--branch-headers () "Return headers for branch-related information." (let ((branch (vc-git--out-match @@ -724,25 +784,38 @@ vc-git-dir--branch-headers "^\\(refs/heads/\\)?\\(.+\\)$" 2)) tracking remote-url) (if branch - (when-let ((branch-merge - (vc-git--out-match - `("config" ,(concat "branch." branch ".merge")) - "^\\(refs/heads/\\)?\\(.+\\)$" 2)) - (branch-remote - (vc-git--out-match - `("config" ,(concat "branch." branch ".remote")) - "\\([^\n]+\\)" 1))) - (if (string= branch-remote ".") - (setq tracking branch-merge - remote-url "none (tracking local branch)") - (setq tracking (concat branch-remote "/" branch-merge) - remote-url (vc-git-repository-url - default-directory branch-remote)))) + (let ((branch-merge + (vc-git--out-match + `("config" ,(concat "branch." branch ".merge")) + "^\\(refs/heads/\\)?\\(.+\\)$" 2)) + (branch-remote + (vc-git--out-match + `("config" ,(concat "branch." branch ".remote")) + "\\([^\n]+\\)" 1))) + ;; Either BRANCH-MERGE and BRANCH-REMOTE are both set, or + ;; neither are. + (cl-assert + (eq (not (not branch-merge)) + (not (not branch-remote))) + nil "Inconsistent branch settings: merge is %s; remote is %s" + branch-merge branch-remote) + (setq tracking (vc-git-dir--tracking + branch branch-merge branch-remote) + remote-url (and branch-remote + (if (equal branch-remote ".") + "none (tracking local branch)" + (vc-git-repository-url + default-directory branch-remote))))) (setq branch "none (detached HEAD)")) (cl-flet ((fmt (key value) (concat (propertize (format "% -11s: " key) 'face 'vc-dir-header) (propertize value 'face 'vc-dir-header-value)))) + (when (and tracking + (eq (alist-get 'how vc-git-dir-show-tracking 'header) + 'inline)) + (setq branch (format "%s (tracking %s)" branch tracking) + tracking nil)) (remove nil (list (fmt "Branch" branch) (and tracking (fmt "Tracking" tracking)) diff --git a/test/lisp/vc/vc-git-tests.el b/test/lisp/vc/vc-git-tests.el index 2dbf5a8df12..4ece262564e 100644 --- a/test/lisp/vc/vc-git-tests.el +++ b/test/lisp/vc/vc-git-tests.el @@ -119,29 +119,34 @@ vc-git-test--start-branch (vc-git-test--run "commit" "-mFirst") (string-trim (vc-git-test--run "branch" "--show-current"))) -(defun vc-git-test--dir-headers (headers) +(defun vc-git-test--dir-headers (headers &optional show-tracking) "Return an alist of header values for the current `vc-dir' buffer. HEADERS should be a list of (NAME ...) strings. This function will return a list of (NAME . VALUE) pairs, where VALUE is nil if the header -is absent." - ;; FIXME: to reproduce interactive sessions faithfully, we would need - ;; to wait for the dir-status-files process to terminate; have not - ;; found a reliable way to do this. As a workaround, kill pending - ;; processes and revert the `vc-dir' buffer. - (vc-dir-kill-dir-status-process) - (revert-buffer) - (mapcar - (lambda (header) - (let* ((pattern - (rx bol - (literal header) (* space) ": " (group (+ nonl)) - eol)) - (value (and (goto-char (point-min)) - (re-search-forward pattern nil t) - (match-string 1)))) - (cons header value))) - headers)) +is absent. + +SHOW-TRACKING is a temporary value to bind `vc-git-dir-show-tracking' +to. If omitted, the default value will be kept." + (let ((vc-git-dir-show-tracking (or show-tracking + vc-git-dir-show-tracking))) + ;; FIXME: to reproduce interactive sessions faithfully, we would need + ;; to wait for the dir-status-files process to terminate; have not + ;; found a reliable way to do this. As a workaround, kill pending + ;; processes and revert the `vc-dir' buffer. + (vc-dir-kill-dir-status-process) + (revert-buffer) + (mapcar + (lambda (header) + (let* ((pattern + (rx bol + (literal header) (* space) ": " (group (+ nonl)) + eol)) + (value (and (goto-char (point-min)) + (re-search-forward pattern nil t) + (match-string 1)))) + (cons header value))) + headers))) (ert-deftest vc-git-test-dir-branch-headers () "Check that `vc-dir' shows expected branch-related headers." @@ -153,6 +158,8 @@ vc-git-test-dir-branch-headers (ert-with-temp-directory clone-repo (vc-git-test--run "clone" origin-repo clone-repo) (vc-dir clone-repo) + + ;; Post-clone: on MAIN-BRANCH, tracking origin/MAIN-BRANCH. (should (equal (vc-git-test--dir-headers @@ -160,6 +167,25 @@ vc-git-test-dir-branch-headers `(("Branch" . ,main-branch) ("Tracking" . ,(concat "origin/" main-branch)) ("Remote" . ,origin-repo)))) + (should + (equal + (vc-git-test--dir-headers + '("Branch" "Tracking") '((how . inline))) + `(("Branch" . ,(format "%s (tracking origin/%s)" main-branch main-branch)) + ("Tracking" . nil)))) + (should + (equal + (vc-git-test--dir-headers + '("Branch" "Tracking") '((when . different))) + `(("Branch" . ,main-branch) + ("Tracking" . nil)))) + (should + (equal + (vc-git-test--dir-headers + '("Branch" "Tracking") '((when . never))) + `(("Branch" . ,main-branch) + ("Tracking" . nil)))) + ;; Checkout a new branch: no tracking information. (vc-git-test--run "checkout" "-b" "feature/foo" main-branch) (should @@ -169,6 +195,13 @@ vc-git-test-dir-branch-headers '(("Branch" . "feature/foo") ("Tracking" . nil) ("Remote" . nil)))) + (should + (equal + (vc-git-test--dir-headers + '("Branch" "Tracking") '((when . t))) + '(("Branch" . "feature/foo") + ("Tracking" . "none")))) + ;; Push with '--set-upstream origin': tracking information ;; should be updated. (vc-git-test--run "push" "--set-upstream" "origin" "feature/foo") @@ -179,6 +212,7 @@ vc-git-test-dir-branch-headers `(("Branch" . "feature/foo") ("Tracking" . "origin/feature/foo") ("Remote" . ,origin-repo)))) + ;; Checkout a new branch tracking the _local_ main branch. ;; Bug#68183. (vc-git-test--run "checkout" "-b" "feature/bar" "--track" main-branch) -- 2.39.2 [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #5: squashed.patch --] [-- Type: text/x-diff, Size: 23520 bytes --] From 2de70ea79a5d8eb98e0dcd4ef0598a8e033b6526 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Le=20Gouguec?= <kevin.legouguec@gmail.com> Date: Sun, 7 Jul 2024 12:16:12 +0200 Subject: [PATCH] Let users choose when and how to display Git tracking branch While in there, split vc-git-dir-extra-headers into more manageable chunks. The current code requires a lot of eyeballing back-and-forth to: - check where variables are actually used, what impact changing them can have: in actuality, there are three distinct "groups" of headers we compute, each with their own independent state; - understand formatting details such as "who's in charge of the newlines". To solve both issues, split that function into smaller ones, each handling a "group" of headers. The only expected "functional" change is that, by propertizing "\nHeader: " strings, the original code sometimes applied the vc-dir-header face to the newline preceding a header; the new code applies no faces to these newlines. This change would be visible to users with themes adding an :extended background to vc-dir-header. In practice, no in-tree theme is impacted. For bug#68183. * lisp/vc/vc-git.el (vc-git-dir-show-tracking): New option. (vc-git-dir--tracking): New function to format upstream branch according to the new option. (vc-git-dir--branch-headers): New function to compute "Branch", "Tracking" and "Remote". (vc-git--cmds-in-progress): Rename to... (vc-git-dir--in-progress-headers): ... this, and compute headers. (vc-git-dir--stash-headers): New function to compute the "Stash" header. (vc-git-dir-extra-headers): Boil down to just setting default-directory and assembling the headers from these new helpers. (vc-git--out-match): New function to call 'git' and capture specific bits of output. * test/lisp/vc/vc-git-tests.el (vc-git-test-dir-track-local-branch): Remove in favor of new test. (vc-git-test--start-branch): New helper to get a repository going. (vc-git-test--dir-headers): New helper to get a list of headers in the current vc-dir buffer. (vc-git-test-dir-branch-headers): New test, exercising the original bug recipe plus more common scenarios, as well as some option tweaks. --- lisp/vc/vc-git.el | 326 ++++++++++++++++++++++------------- test/lisp/vc/vc-git-tests.el | 132 ++++++++++++-- 2 files changed, 318 insertions(+), 140 deletions(-) diff --git a/lisp/vc/vc-git.el b/lisp/vc/vc-git.el index e8257c5dbd0..86752bd074d 100644 --- a/lisp/vc/vc-git.el +++ b/lisp/vc/vc-git.el @@ -717,6 +717,136 @@ vc-git-dir-status-files :files files :update-function update-function))) +(defcustom vc-git-dir-show-tracking '((when . set) + (how . header)) + "Control how `vc-dir' shows the upstream branch. +The \"upstream\" branch is the one `vc-pull' fetches changes from by +default. In Git terms, when checking out branch B, the upstream branch +is defined by the configuration options branch.B.merge and +branch.B.remote. + +This option is an alist which admits the following symbol keys: + +* `when' controls whether information about the upstream branch will be + shown. The value for this key can be one of the following symbols: + - set (default) Only show the upstream branch if it is set, + as defined by the previously mentioned Git config + options. + - t If a branch is checked out (that is, HEAD is not + detached), always show something: fallback to \"none\" + if the current branch is not tracking anything. + - different Only show the upstream branch if branch.B.merge is + named differently from B. This allows hiding the + header in the common case where branch \"foo\" tracks + \"origin/foo\". + - never Never show the upstream branch. + +* `how' controls the way this information will be shown. The value can + be one of the following symbols: + - header (default) Show the branch in a dedicated header, + \"Tracking\". + - inline Append the branch to the \"Branch\" header, e.g. + Branch: foo (tracking origin/bar)" + :type 'alist + :options + '((when (radio + (const :tag "Never" never) + (const :tag "Always" t) + (const :tag "If current branch has a tracking branch" set) + (const :tag "If current & tracking branches have different names" different))) + (how (radio + (const :tag "\"Tracking\" header" header) + (const :tag "Inline in \"Branch\" header" inline)))) + :version "31.1") + +(defun vc-git-dir--tracking (branch branch-merge branch-remote) + "Return a description of BRANCH's upstream branch. +This description heeds `vc-git-dir-show-tracking'." + (cl-flet ((remote-prefix () + (if (equal branch-remote ".") + nil + (concat branch-remote "/")))) + (pcase-exhaustive (alist-get 'when vc-git-dir-show-tracking 'set) + ('set (and branch-merge + (concat (remote-prefix) branch-merge))) + ('never nil) + ('t (if branch-merge + (concat (remote-prefix) branch-merge) + "none")) + ('different (and branch-merge + (not (equal branch branch-merge)) + (concat (remote-prefix) branch-merge)))))) + +(defun vc-git-dir--branch-headers () + "Return headers for branch-related information." + (let ((branch (vc-git--out-match + '("symbolic-ref" "HEAD") + "^\\(refs/heads/\\)?\\(.+\\)$" 2)) + tracking remote-url) + (if branch + (let ((branch-merge + (vc-git--out-match + `("config" ,(concat "branch." branch ".merge")) + "^\\(refs/heads/\\)?\\(.+\\)$" 2)) + (branch-remote + (vc-git--out-match + `("config" ,(concat "branch." branch ".remote")) + "\\([^\n]+\\)" 1))) + ;; Either BRANCH-MERGE and BRANCH-REMOTE are both set, or + ;; neither are. + (cl-assert + (eq (not (not branch-merge)) + (not (not branch-remote))) + nil "Inconsistent branch settings: merge is %s; remote is %s" + branch-merge branch-remote) + (setq tracking (vc-git-dir--tracking + branch branch-merge branch-remote) + remote-url (and branch-remote + (if (equal branch-remote ".") + "none (tracking local branch)" + (vc-git-repository-url + default-directory branch-remote))))) + (setq branch "none (detached HEAD)")) + (cl-flet ((fmt (key value) + (concat + (propertize (format "% -11s: " key) 'face 'vc-dir-header) + (propertize value 'face 'vc-dir-header-value)))) + (when (and tracking + (eq (alist-get 'how vc-git-dir-show-tracking 'header) + 'inline)) + (setq branch (format "%s (tracking %s)" branch tracking) + tracking nil)) + (remove nil (list + (fmt "Branch" branch) + (and tracking (fmt "Tracking" tracking)) + (and remote-url (fmt "Remote" remote-url))))))) + +(defun vc-git-dir--in-progress-headers () + "Return headers for Git commands in progress in this worktree." + (let ((gitdir (vc-git--git-path)) + cmds) + ;; See contrib/completion/git-prompt.sh in git.git. + (when (or (file-directory-p + (expand-file-name "rebase-merge" gitdir)) + (file-exists-p + (expand-file-name "rebase-apply/rebasing" gitdir))) + (push 'rebase cmds)) + (when (file-exists-p + (expand-file-name "rebase-apply/applying" gitdir)) + (push 'am cmds)) + (when (file-exists-p (expand-file-name "MERGE_HEAD" gitdir)) + (push 'merge cmds)) + (when (file-exists-p (expand-file-name "BISECT_START" gitdir)) + (push 'bisect cmds)) + (cl-flet ((fmt (cmd name) + (when (memq cmd cmds) + ;; For now just a heading, key bindings can be added + ;; later for various bisect actions. + (propertize (format "% -11s: in progress" name) + 'face 'vc-dir-status-warning)))) + (remove nil (list (fmt 'bisect "Bisect") + (fmt 'rebase "Rebase")))))) + (defvar-keymap vc-git-stash-shared-map "S" #'vc-git-stash-snapshot "C" #'vc-git-stash) @@ -797,130 +927,75 @@ vc-git-stash-menu-map :help "Show the contents of the current stash")) map)) -(defun vc-git--cmds-in-progress () - "Return a list of Git commands in progress in this worktree." - (let ((gitdir (vc-git--git-path)) - cmds) - ;; See contrib/completion/git-prompt.sh in git.git. - (when (or (file-directory-p - (expand-file-name "rebase-merge" gitdir)) - (file-exists-p - (expand-file-name "rebase-apply/rebasing" gitdir))) - (push 'rebase cmds)) - (when (file-exists-p - (expand-file-name "rebase-apply/applying" gitdir)) - (push 'am cmds)) - (when (file-exists-p (expand-file-name "MERGE_HEAD" gitdir)) - (push 'merge cmds)) - (when (file-exists-p (expand-file-name "BISECT_START" gitdir)) - (push 'bisect cmds)) - cmds)) +(defun vc-git-dir--stash-headers () + "Return headers describing the current stashes." + (list + (concat + (propertize "Stash : " 'face 'vc-dir-header) + (if-let ((stash-list (vc-git-stash-list))) + (let* ((len (length stash-list)) + (limit + (if (integerp vc-git-show-stash) + (min vc-git-show-stash len) + len)) + (shown-stashes (cl-subseq stash-list 0 limit)) + (hidden-stashes (cl-subseq stash-list limit)) + (all-hideable (or (eq vc-git-show-stash t) + (<= len vc-git-show-stash)))) + (concat + ;; Button to toggle visibility. + (if all-hideable + (vc-git-make-stash-button nil limit limit) + (vc-git-make-stash-button t vc-git-show-stash len)) + ;; Stash list. + (when shown-stashes + (concat + (propertize "\n" + 'vc-git-hideable all-hideable) + (mapconcat + (lambda (x) + (propertize x + 'face 'vc-dir-header-value + 'mouse-face 'highlight + 'vc-git-hideable all-hideable + 'help-echo vc-git-stash-list-help + 'keymap vc-git-stash-map)) + shown-stashes + (propertize "\n" + 'vc-git-hideable all-hideable)))) + (when hidden-stashes + (concat + (propertize "\n" + 'invisible t + 'vc-git-hideable t) + (mapconcat + (lambda (x) + (propertize x + 'face 'vc-dir-header-value + 'mouse-face 'highlight + 'invisible t + 'vc-git-hideable t + 'help-echo vc-git-stash-list-help + 'keymap vc-git-stash-map)) + hidden-stashes + (propertize "\n" + 'invisible t + 'vc-git-hideable t)))))) + (propertize "Nothing stashed" + 'help-echo vc-git-stash-shared-help + 'keymap vc-git-stash-shared-map + 'face 'vc-dir-header-value))))) (defun vc-git-dir-extra-headers (dir) - (let ((str (vc-git--out-str "symbolic-ref" "HEAD")) - (stash-list (vc-git-stash-list)) - (default-directory dir) - (in-progress (vc-git--cmds-in-progress)) - - branch remote-url stash-button stash-string tracking-branch) - (if (string-match "^\\(refs/heads/\\)?\\(.+\\)$" str) - (progn - (setq branch (match-string 2 str)) - (let ((remote (vc-git--out-str - "config" (concat "branch." branch ".remote"))) - (merge (vc-git--out-str - "config" (concat "branch." branch ".merge")))) - (when (string-match "\\([^\n]+\\)" remote) - (setq remote (match-string 1 remote))) - (when (string-match "^\\(refs/heads/\\)?\\(.+\\)$" merge) - (setq tracking-branch (match-string 2 merge))) - (pcase remote - ("." - (setq remote-url "none (tracking local branch)")) - ((pred (not string-empty-p)) - (setq - remote-url (vc-git-repository-url dir remote) - tracking-branch (concat remote "/" tracking-branch)))))) - (setq branch "none (detached HEAD)")) - (when stash-list - (let* ((len (length stash-list)) - (limit - (if (integerp vc-git-show-stash) - (min vc-git-show-stash len) - len)) - (shown-stashes (cl-subseq stash-list 0 limit)) - (hidden-stashes (cl-subseq stash-list limit)) - (all-hideable (or (eq vc-git-show-stash t) - (<= len vc-git-show-stash)))) - (setq stash-button (if all-hideable - (vc-git-make-stash-button nil limit limit) - (vc-git-make-stash-button t vc-git-show-stash len)) - stash-string - (concat - (when shown-stashes - (concat - (propertize "\n" - 'vc-git-hideable all-hideable) - (mapconcat - (lambda (x) - (propertize x - 'face 'vc-dir-header-value - 'mouse-face 'highlight - 'vc-git-hideable all-hideable - 'help-echo vc-git-stash-list-help - 'keymap vc-git-stash-map)) - shown-stashes - (propertize "\n" - 'vc-git-hideable all-hideable)))) - (when hidden-stashes - (concat - (propertize "\n" - 'invisible t - 'vc-git-hideable t) - (mapconcat - (lambda (x) - (propertize x - 'face 'vc-dir-header-value - 'mouse-face 'highlight - 'invisible t - 'vc-git-hideable t - 'help-echo vc-git-stash-list-help - 'keymap vc-git-stash-map)) - hidden-stashes - (propertize "\n" - 'invisible t - 'vc-git-hideable t)))))))) - (concat - (propertize "Branch : " 'face 'vc-dir-header) - (propertize branch - 'face 'vc-dir-header-value) - (when tracking-branch - (concat - "\n" - (propertize "Tracking : " 'face 'vc-dir-header) - (propertize tracking-branch 'face 'vc-dir-header-value))) - (when remote-url - (concat - "\n" - (propertize "Remote : " 'face 'vc-dir-header) - (propertize remote-url - 'face 'vc-dir-header-value))) - ;; For now just a heading, key bindings can be added later for various bisect actions - (when (memq 'bisect in-progress) - (propertize "\nBisect : in progress" 'face 'vc-dir-status-warning)) - (when (memq 'rebase in-progress) - (propertize "\nRebase : in progress" 'face 'vc-dir-status-warning)) - (if stash-list - (concat - (propertize "\nStash : " 'face 'vc-dir-header) - stash-button - stash-string) - (concat - (propertize "\nStash : " 'face 'vc-dir-header) - (propertize "Nothing stashed" - 'help-echo vc-git-stash-shared-help - 'keymap vc-git-stash-shared-map - 'face 'vc-dir-header-value)))))) + (let ((default-directory dir)) + (string-join + (append + ;; Each helper returns a list of headers. Each header must be a + ;; propertized string with no final newline. + (vc-git-dir--branch-headers) + (vc-git-dir--in-progress-headers) + (vc-git-dir--stash-headers)) + "\n"))) (defun vc-git-branches () "Return the existing branches, as a list of strings. @@ -2246,6 +2321,13 @@ vc-git--out-str (with-current-buffer standard-output (apply #'vc-git--out-ok command args)))) +(defun vc-git--out-match (args regexp group) + "Run `git ARGS...' and return match for group number GROUP of REGEXP. +Return nil if the output does not match. The exit status is ignored." + (let ((out (apply #'vc-git--out-str args))) + (when (string-match regexp out) + (match-string group out)))) + (defun vc-git--run-command-string (file &rest args) "Run a git command on FILE and return its output as string. FILE can be nil." diff --git a/test/lisp/vc/vc-git-tests.el b/test/lisp/vc/vc-git-tests.el index f15a0f52e8c..4ece262564e 100644 --- a/test/lisp/vc/vc-git-tests.el +++ b/test/lisp/vc/vc-git-tests.el @@ -26,6 +26,7 @@ (require 'ert-x) (require 'vc) +(require 'vc-dir) (require 'vc-git) (ert-deftest vc-git-test-program-version-general () @@ -108,24 +109,119 @@ vc-git-test--run (apply 'vc-git-command t 0 nil args) (buffer-string))) -(ert-deftest vc-git-test-dir-track-local-branch () - "Test that `vc-dir' works when tracking local branches. Bug#68183." +(defun vc-git-test--start-branch () + "Get a branch started in a freshly initialized repository. + +This returns the name of the current branch, so that tests can remain +agnostic of init.defaultbranch." + (write-region "hello" nil "README") + (vc-git-test--run "add" "README") + (vc-git-test--run "commit" "-mFirst") + (string-trim (vc-git-test--run "branch" "--show-current"))) + +(defun vc-git-test--dir-headers (headers &optional show-tracking) + "Return an alist of header values for the current `vc-dir' buffer. + +HEADERS should be a list of (NAME ...) strings. This function will +return a list of (NAME . VALUE) pairs, where VALUE is nil if the header +is absent. + +SHOW-TRACKING is a temporary value to bind `vc-git-dir-show-tracking' +to. If omitted, the default value will be kept." + (let ((vc-git-dir-show-tracking (or show-tracking + vc-git-dir-show-tracking))) + ;; FIXME: to reproduce interactive sessions faithfully, we would need + ;; to wait for the dir-status-files process to terminate; have not + ;; found a reliable way to do this. As a workaround, kill pending + ;; processes and revert the `vc-dir' buffer. + (vc-dir-kill-dir-status-process) + (revert-buffer) + (mapcar + (lambda (header) + (let* ((pattern + (rx bol + (literal header) (* space) ": " (group (+ nonl)) + eol)) + (value (and (goto-char (point-min)) + (re-search-forward pattern nil t) + (match-string 1)))) + (cons header value))) + headers))) + +(ert-deftest vc-git-test-dir-branch-headers () + "Check that `vc-dir' shows expected branch-related headers." (skip-unless (executable-find vc-git-program)) - (vc-git-test--with-repo repo - ;; Create an initial commit to get a branch started. - (write-region "hello" nil "README") - (vc-git-test--run "add" "README") - (vc-git-test--run "commit" "-mFirst") - ;; Get current branch name lazily, to remain agnostic of - ;; init.defaultbranch. - (let ((upstream-branch - (string-trim (vc-git-test--run "branch" "--show-current")))) - (vc-git-test--run "checkout" "--track" "-b" "hack" upstream-branch) - (vc-dir default-directory) - (pcase-dolist (`(,header ,value) - `(("Branch" "hack") - ("Tracking" ,upstream-branch))) - (goto-char (point-min)) - (re-search-forward (format "^%s *: %s$" header value)))))) + ;; Create a repository that will serve as the "remote". + (vc-git-test--with-repo origin-repo + (let ((main-branch (vc-git-test--start-branch))) + ;; 'git clone' this repository and test things in this clone. + (ert-with-temp-directory clone-repo + (vc-git-test--run "clone" origin-repo clone-repo) + (vc-dir clone-repo) + + ;; Post-clone: on MAIN-BRANCH, tracking origin/MAIN-BRANCH. + (should + (equal + (vc-git-test--dir-headers + '("Branch" "Tracking" "Remote")) + `(("Branch" . ,main-branch) + ("Tracking" . ,(concat "origin/" main-branch)) + ("Remote" . ,origin-repo)))) + (should + (equal + (vc-git-test--dir-headers + '("Branch" "Tracking") '((how . inline))) + `(("Branch" . ,(format "%s (tracking origin/%s)" main-branch main-branch)) + ("Tracking" . nil)))) + (should + (equal + (vc-git-test--dir-headers + '("Branch" "Tracking") '((when . different))) + `(("Branch" . ,main-branch) + ("Tracking" . nil)))) + (should + (equal + (vc-git-test--dir-headers + '("Branch" "Tracking") '((when . never))) + `(("Branch" . ,main-branch) + ("Tracking" . nil)))) + + ;; Checkout a new branch: no tracking information. + (vc-git-test--run "checkout" "-b" "feature/foo" main-branch) + (should + (equal + (vc-git-test--dir-headers + '("Branch" "Tracking" "Remote")) + '(("Branch" . "feature/foo") + ("Tracking" . nil) + ("Remote" . nil)))) + (should + (equal + (vc-git-test--dir-headers + '("Branch" "Tracking") '((when . t))) + '(("Branch" . "feature/foo") + ("Tracking" . "none")))) + + ;; Push with '--set-upstream origin': tracking information + ;; should be updated. + (vc-git-test--run "push" "--set-upstream" "origin" "feature/foo") + (should + (equal + (vc-git-test--dir-headers + '("Branch" "Tracking" "Remote")) + `(("Branch" . "feature/foo") + ("Tracking" . "origin/feature/foo") + ("Remote" . ,origin-repo)))) + + ;; Checkout a new branch tracking the _local_ main branch. + ;; Bug#68183. + (vc-git-test--run "checkout" "-b" "feature/bar" "--track" main-branch) + (should + (equal + (vc-git-test--dir-headers + '("Branch" "Tracking" "Remote")) + `(("Branch" . "feature/bar") + ("Tracking" . ,main-branch) + ("Remote" . "none (tracking local branch)")))))))) ;;; vc-git-tests.el ends here -- 2.39.2 ^ permalink raw reply related [flat|nested] 29+ messages in thread
* bug#68183: 28.3; vc-dir fails when I have a certain branch checked out 2024-08-07 14:25 ` Kévin Le Gouguec @ 2024-08-08 0:32 ` Sean Whitton 2024-08-08 7:07 ` Kévin Le Gouguec 2024-08-13 1:32 ` Dmitry Gutov 1 sibling, 1 reply; 29+ messages in thread From: Sean Whitton @ 2024-08-08 0:32 UTC (permalink / raw) To: Kévin Le Gouguec Cc: Dmitry Gutov, Eli Zaretskii, 68183, Tom Tromey, Juri Linkov Hello, On Wed 07 Aug 2024 at 04:25pm +02, Kévin Le Gouguec wrote: > About patch #2, CC'ing Sean Whitton for perspective on > vc-git--cmds-in-progress: I was puzzled by the function supporting many > commands (rebase, am, merge, bisect), whereas AFAICT its sole user only > heeds 'bisect & 'rebase. Wondering if I've missed other in-tree uses, > or if we should add headers for 'am and 'merge, "while in there". I have some WIP which uses this function for some other purposes. It's a pretty cleanly identifiable piece of functionality so I would like to keep it separate. I would be happy if you were to add detecting some other operations there, for sure, if you are confident in your detection methods. -- Sean Whitton ^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#68183: 28.3; vc-dir fails when I have a certain branch checked out 2024-08-08 0:32 ` Sean Whitton @ 2024-08-08 7:07 ` Kévin Le Gouguec 2024-08-08 12:02 ` Sean Whitton 0 siblings, 1 reply; 29+ messages in thread From: Kévin Le Gouguec @ 2024-08-08 7:07 UTC (permalink / raw) To: Sean Whitton; +Cc: Dmitry Gutov, Eli Zaretskii, 68183, Tom Tromey, Juri Linkov Sean Whitton <spwhitton@spwhitton.name> writes: >> About patch #2, CC'ing Sean Whitton for perspective on >> vc-git--cmds-in-progress: I was puzzled by the function supporting many >> commands (rebase, am, merge, bisect), whereas AFAICT its sole user only >> heeds 'bisect & 'rebase. Wondering if I've missed other in-tree uses, >> or if we should add headers for 'am and 'merge, "while in there". > > I have some WIP which uses this function for some other purposes. > It's a pretty cleanly identifiable piece of functionality so I would > like to keep it separate. Gotcha, thanks for weighing in! Then I'll work on another revision of the series to keep that function intact. > I would be happy if you were to add detecting some other operations > there, for sure, if you are confident in your detection methods. Do you mean that the detection methods for 'am and 'merge currently in place… (when (file-exists-p (expand-file-name "rebase-apply/applying" gitdir)) (push 'am cmds)) (when (file-exists-p (expand-file-name "MERGE_HEAD" gitdir)) (push 'merge cmds)) … are unreliable somehow? I wasn't planning on detecting more operations, merely adding vc-dir headers for these two, since despite vc-git--cmds-in-progress picking these operations up, vc-git-dir-extra-headers ignores them 😶 ^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#68183: 28.3; vc-dir fails when I have a certain branch checked out 2024-08-08 7:07 ` Kévin Le Gouguec @ 2024-08-08 12:02 ` Sean Whitton 0 siblings, 0 replies; 29+ messages in thread From: Sean Whitton @ 2024-08-08 12:02 UTC (permalink / raw) To: Kévin Le Gouguec Cc: Dmitry Gutov, Eli Zaretskii, 68183, Tom Tromey, Juri Linkov Hello, On Thu 08 Aug 2024 at 09:07am +02, Kévin Le Gouguec wrote: > Do you mean that the detection methods for 'am and 'merge currently in > place… > > (when (file-exists-p > (expand-file-name "rebase-apply/applying" gitdir)) > (push 'am cmds)) > (when (file-exists-p (expand-file-name "MERGE_HEAD" gitdir)) > (push 'merge cmds)) > > … are unreliable somehow? I wasn't planning on detecting more > operations, merely adding vc-dir headers for these two, since despite > vc-git--cmds-in-progress picking these operations up, > vc-git-dir-extra-headers ignores them 😶 Oh sorry, I misread. Those tests are reliable. Yes -- if you think it would be helpful to have them displayed in vc-dir, it makes sense to me to add them. -- Sean Whitton ^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#68183: 28.3; vc-dir fails when I have a certain branch checked out 2024-08-07 14:25 ` Kévin Le Gouguec 2024-08-08 0:32 ` Sean Whitton @ 2024-08-13 1:32 ` Dmitry Gutov 2024-08-20 6:15 ` Kévin Le Gouguec 1 sibling, 1 reply; 29+ messages in thread From: Dmitry Gutov @ 2024-08-13 1:32 UTC (permalink / raw) To: Kévin Le Gouguec, 68183 Cc: Eli Zaretskii, Tom Tromey, Sean Whitton, Juri Linkov Hi Kevin, On 07/08/2024 17:25, Kévin Le Gouguec wrote: > Heya, > > Have spent cycles on this on-and-off these past few months; finally have > something worth discussing, I think 🤞 > > To recap where we stand, AFAIU: the reported vc-dir bug has been fixed > (in time for the emacs-30 branch), but the changes could feel intrusive, > since a new vc-dir header was added ("Tracking") that some users may not > care for. I haven't looked at the patch in detail, but FWIW the new header feels like a minor enough detail to not be annoying (or even noticeable) when you don't want it, but it's good to have when you do need that info. This is my impression after having that feature around for a few months since it's been introduced anyway. Just a data point. Not to imply that the code couldn't be improved, or that the new option can't be useful. ^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#68183: 28.3; vc-dir fails when I have a certain branch checked out 2024-08-13 1:32 ` Dmitry Gutov @ 2024-08-20 6:15 ` Kévin Le Gouguec 2024-08-20 12:15 ` Eli Zaretskii 2024-08-21 0:42 ` Dmitry Gutov 0 siblings, 2 replies; 29+ messages in thread From: Kévin Le Gouguec @ 2024-08-20 6:15 UTC (permalink / raw) To: Dmitry Gutov; +Cc: Eli Zaretskii, 68183, Tom Tromey, Sean Whitton, Juri Linkov Dmitry Gutov <dmitry@gutov.dev> writes: > Hi Kevin, > > On 07/08/2024 17:25, Kévin Le Gouguec wrote: >> Heya, >> Have spent cycles on this on-and-off these past few months; finally have >> something worth discussing, I think 🤞 >> To recap where we stand, AFAIU: the reported vc-dir bug has been fixed >> (in time for the emacs-30 branch), but the changes could feel intrusive, >> since a new vc-dir header was added ("Tracking") that some users may not >> care for. > > I haven't looked at the patch in detail, but FWIW the new header feels like a minor enough detail to not be annoying (or even noticeable) when you don't want it, but it's good to have when you do need that info. > > This is my impression after having that feature around for a few months since it's been introduced anyway. > > Just a data point. Not to imply that the code couldn't be improved, or that the new option can't be useful. Thanks for weighing in. Since I am not a frequent vc-dir user, my disposition would then be to avoid committing to sophisticated user options until there is demand for it. So, at this stage, ISTM: * We could apply [patch 1] (new tests) on emacs-30. * I should draft a NEWS entry for the new header, for emacs-30. * We could apply [patch 2] (refactoring; modulo the considerations re. vc-git--cmds-in-progress discussed with Sean) on master, if we feel like it makes vc-git-dir-extra-headers easier to work with. [patch 1]: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=68183;filename=0001-Test-more-vc-dir-scenarios-with-Git-bug-68183.patch;msg=41;att=1 [patch 2]: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=68183;filename=0002-Split-vc-git-dir-extra-headers-into-more-manageable-.patch;msg=41;att=2 ^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#68183: 28.3; vc-dir fails when I have a certain branch checked out 2024-08-20 6:15 ` Kévin Le Gouguec @ 2024-08-20 12:15 ` Eli Zaretskii 2024-08-22 7:15 ` Kévin Le Gouguec 2024-08-21 0:42 ` Dmitry Gutov 1 sibling, 1 reply; 29+ messages in thread From: Eli Zaretskii @ 2024-08-20 12:15 UTC (permalink / raw) To: Kévin Le Gouguec; +Cc: dmitry, 68183, tom, spwhitton, juri > From: Kévin Le Gouguec <kevin.legouguec@gmail.com> > Cc: 68183@debbugs.gnu.org, Eli Zaretskii <eliz@gnu.org>, Tom Tromey > <tom@tromey.com>, Juri Linkov <juri@linkov.net>, Sean Whitton > <spwhitton@spwhitton.name> > Date: Tue, 20 Aug 2024 08:15:37 +0200 > > Dmitry Gutov <dmitry@gutov.dev> writes: > > > Hi Kevin, > > > > On 07/08/2024 17:25, Kévin Le Gouguec wrote: > >> Heya, > >> Have spent cycles on this on-and-off these past few months; finally have > >> something worth discussing, I think 🤞 > >> To recap where we stand, AFAIU: the reported vc-dir bug has been fixed > >> (in time for the emacs-30 branch), but the changes could feel intrusive, > >> since a new vc-dir header was added ("Tracking") that some users may not > >> care for. > > > > I haven't looked at the patch in detail, but FWIW the new header feels like a minor enough detail to not be annoying (or even noticeable) when you don't want it, but it's good to have when you do need that info. > > > > This is my impression after having that feature around for a few months since it's been introduced anyway. > > > > Just a data point. Not to imply that the code couldn't be improved, or that the new option can't be useful. > > Thanks for weighing in. Since I am not a frequent vc-dir user, my > disposition would then be to avoid committing to sophisticated user > options until there is demand for it. > > So, at this stage, ISTM: > > * We could apply [patch 1] (new tests) on emacs-30. > > * I should draft a NEWS entry for the new header, for emacs-30. > > * We could apply [patch 2] (refactoring; modulo the considerations > re. vc-git--cmds-in-progress discussed with Sean) on master, if we > feel like it makes vc-git-dir-extra-headers easier to work with. > > > [patch 1]: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=68183;filename=0001-Test-more-vc-dir-scenarios-with-Git-bug-68183.patch;msg=41;att=1 > > [patch 2]: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=68183;filename=0002-Split-vc-git-dir-extra-headers-into-more-manageable-.patch;msg=41;att=2 A NEWS entry is probably OK, though I'd like to see it first. As for adding tests, I think they should go to master. There's no purpose in adding them to the release branch, which is basically frozen to code changes except bugfixes. Since we merge code to master routinely, any regression that happens on the branch will soon enough land on master and will be detected there. Thanks. ^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#68183: 28.3; vc-dir fails when I have a certain branch checked out 2024-08-20 12:15 ` Eli Zaretskii @ 2024-08-22 7:15 ` Kévin Le Gouguec 2024-08-22 12:46 ` Eli Zaretskii 0 siblings, 1 reply; 29+ messages in thread From: Kévin Le Gouguec @ 2024-08-22 7:15 UTC (permalink / raw) To: Eli Zaretskii; +Cc: dmitry, juri, 68183, tom, spwhitton [-- Attachment #1: Type: text/plain, Size: 1268 bytes --] Eli Zaretskii <eliz@gnu.org> writes: > A NEWS entry is probably OK, though I'd like to see it first. Please find a draft attached. Lifted the "upstream branch" definition from the horse's mouth, i.e. gitglossary(7), adjusting for VC relevance, i.e. "where vc-pull fetches". (Agonized a bit over how to typeset "VC-dir buffers"; went with that spelling because a bunch of vc-dir.el deffaces use "VC-dir". *5 seconds later* But I now realize etc/NEWSes also use "VC Dired", "VC-dired", "vc-dir for <backend>", "*vc-dir*", "VC directory mode", vc-dir-mode, and "VC directory buffer", so maybe I oughtn't worry so much? *5 seconds later* 💡 Ah, well, guess I could follow The Fine Manual's nomenclature? Changed to "VC Directory buffers", per '(emacs) VC Directory Mode'. Kept the shorthand for the ChangeLog message for brevity) In another subthread, Sean Whitton <spwhitton@spwhitton.name> writes: > (I did s/find some use/find use/ in your log message because it seemed > more(?) grammatical. Now I read it again, I'm not sure that is true. > I hope this is okay with you. It's a very small change.) (Can't attest to grammaticality, but AFAICT you ditched a weasel word and reduced noise, and that's quite OK, thanks!) [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: 0001-etc-NEWS-Announce-VC-dir-Tracking-header.-bug-68183.patch --] [-- Type: text/x-patch, Size: 1136 bytes --] From 6c918ded7ebded709550dd87ce3e0e1688c7446a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Le=20Gouguec?= <kevin.legouguec@gmail.com> Date: Thu, 22 Aug 2024 08:34:03 +0200 Subject: [PATCH] ; * etc/NEWS: Announce VC-dir "Tracking" header. (bug#68183) --- etc/NEWS | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/etc/NEWS b/etc/NEWS index 98eca7b8061..1fb23554f59 100644 --- a/etc/NEWS +++ b/etc/NEWS @@ -1017,6 +1017,16 @@ behavior, set user option 'vc-annotate-use-short-revision' to nil. *** New user option 'vc-git-file-name-changes-switches'. It allows tweaking the thresholds for rename and copy detection. +--- +*** VC Directory buffers now display the upstream branch in Git repositories. +The "upstream branch" is the branch 'vc-pull' fetches changes from by +default. In Git terms, the upstream branch of branch B is determined by +configuration variables branch.B.remote and branch.B.merge. + +When these configuration variables are set for the current branch, the +VC Directory buffer will show the corresponding upstream branch under +the "Tracking" header. + ** Diff mode --- -- 2.46.0 ^ permalink raw reply related [flat|nested] 29+ messages in thread
* bug#68183: 28.3; vc-dir fails when I have a certain branch checked out 2024-08-22 7:15 ` Kévin Le Gouguec @ 2024-08-22 12:46 ` Eli Zaretskii 2024-08-29 15:36 ` Kévin Le Gouguec 0 siblings, 1 reply; 29+ messages in thread From: Eli Zaretskii @ 2024-08-22 12:46 UTC (permalink / raw) To: Kévin Le Gouguec; +Cc: dmitry, juri, 68183, tom, spwhitton > From: Kévin Le Gouguec <kevin.legouguec@gmail.com> > Cc: dmitry@gutov.dev, 68183@debbugs.gnu.org, tom@tromey.com, > spwhitton@spwhitton.name, juri@linkov.net > Date: Thu, 22 Aug 2024 09:15:06 +0200 > > +--- > +*** VC Directory buffers now display the upstream branch in Git repositories. > +The "upstream branch" is the branch 'vc-pull' fetches changes from by > +default. In Git terms, the upstream branch of branch B is determined by > +configuration variables branch.B.remote and branch.B.merge. That's okay, with the following nit: I find phrases like "the branch 'vc-pull' fetches changes from by default" hard to grasp. I prefer rephrasing to make the grammar simpler: The "upstream branch" is the branch from which 'vc-pull' fetches changes by default. ^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#68183: 28.3; vc-dir fails when I have a certain branch checked out 2024-08-22 12:46 ` Eli Zaretskii @ 2024-08-29 15:36 ` Kévin Le Gouguec 2024-08-29 15:46 ` Eli Zaretskii 0 siblings, 1 reply; 29+ messages in thread From: Kévin Le Gouguec @ 2024-08-29 15:36 UTC (permalink / raw) To: Eli Zaretskii; +Cc: dmitry, 68183, tom, spwhitton, juri [-- Attachment #1: Type: text/plain, Size: 987 bytes --] Eli Zaretskii <eliz@gnu.org> writes: >> +--- >> +*** VC Directory buffers now display the upstream branch in Git repositories. >> +The "upstream branch" is the branch 'vc-pull' fetches changes from by >> +default. In Git terms, the upstream branch of branch B is determined by >> +configuration variables branch.B.remote and branch.B.merge. > > That's okay, with the following nit: I find phrases like "the branch > 'vc-pull' fetches changes from by default" hard to grasp. I prefer > rephrasing to make the grammar simpler: > > The "upstream branch" is the branch from which 'vc-pull' fetches > changes by default. ACK; got the attached at the tip of my emacs-30 branch, and my finger over the 'git push' button. Let me know if it's time to 'C-x v P'. (And mark this bug "done", since I believe this was the last loose end?) (Prefer asking since I let a week fly by, on the off chance that the release branch might have reached a "hands off ✋" stage) [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: 0001-etc-NEWS-Announce-VC-dir-Tracking-header.-bug-68183.patch --] [-- Type: text/x-diff, Size: 1142 bytes --] From 39ae8a21b342acf7d60945db4d36dcabb52385e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Le=20Gouguec?= <kevin.legouguec@gmail.com> Date: Thu, 22 Aug 2024 08:34:03 +0200 Subject: [PATCH] ; * etc/NEWS: Announce VC-dir "Tracking" header. (bug#68183) --- etc/NEWS | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/etc/NEWS b/etc/NEWS index 98eca7b8061..9cf41188868 100644 --- a/etc/NEWS +++ b/etc/NEWS @@ -1017,6 +1017,16 @@ behavior, set user option 'vc-annotate-use-short-revision' to nil. *** New user option 'vc-git-file-name-changes-switches'. It allows tweaking the thresholds for rename and copy detection. +--- +*** VC Directory buffers now display the upstream branch in Git repositories. +The "upstream branch" is the branch from which 'vc-pull' fetches changes +by default. In Git terms, the upstream branch of branch B is determined +by configuration variables branch.B.remote and branch.B.merge. + +When these configuration variables are set for the current branch, the +VC Directory buffer will show the corresponding upstream branch under +the "Tracking" header. + ** Diff mode --- -- 2.39.2 ^ permalink raw reply related [flat|nested] 29+ messages in thread
* bug#68183: 28.3; vc-dir fails when I have a certain branch checked out 2024-08-29 15:36 ` Kévin Le Gouguec @ 2024-08-29 15:46 ` Eli Zaretskii 2024-08-29 16:41 ` Kévin Le Gouguec 0 siblings, 1 reply; 29+ messages in thread From: Eli Zaretskii @ 2024-08-29 15:46 UTC (permalink / raw) To: Kévin Le Gouguec; +Cc: dmitry, 68183, tom, spwhitton, juri > From: Kévin Le Gouguec <kevin.legouguec@gmail.com> > Cc: dmitry@gutov.dev, juri@linkov.net, 68183@debbugs.gnu.org, > tom@tromey.com, spwhitton@spwhitton.name > Date: Thu, 29 Aug 2024 16:36:28 +0100 > > ACK; got the attached at the tip of my emacs-30 branch, and my finger > over the 'git push' button. Let me know if it's time to 'C-x v P'. > (And mark this bug "done", since I believe this was the last loose end?) OK, thanks. ^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#68183: 28.3; vc-dir fails when I have a certain branch checked out 2024-08-29 15:46 ` Eli Zaretskii @ 2024-08-29 16:41 ` Kévin Le Gouguec 0 siblings, 0 replies; 29+ messages in thread From: Kévin Le Gouguec @ 2024-08-29 16:41 UTC (permalink / raw) To: Eli Zaretskii; +Cc: dmitry, juri, tom, 68183-done, spwhitton Eli Zaretskii <eliz@gnu.org> writes: >> From: Kévin Le Gouguec <kevin.legouguec@gmail.com> >> Cc: dmitry@gutov.dev, juri@linkov.net, 68183@debbugs.gnu.org, >> tom@tromey.com, spwhitton@spwhitton.name >> Date: Thu, 29 Aug 2024 16:36:28 +0100 >> >> ACK; got the attached at the tip of my emacs-30 branch, and my finger >> over the 'git push' button. Let me know if it's time to 'C-x v P'. >> (And mark this bug "done", since I believe this was the last loose end?) > > OK, thanks. Thank y'all; pushed, and closing with this message 🙇 ^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#68183: 28.3; vc-dir fails when I have a certain branch checked out 2024-08-20 6:15 ` Kévin Le Gouguec 2024-08-20 12:15 ` Eli Zaretskii @ 2024-08-21 0:42 ` Dmitry Gutov 2024-08-21 1:40 ` Sean Whitton 1 sibling, 1 reply; 29+ messages in thread From: Dmitry Gutov @ 2024-08-21 0:42 UTC (permalink / raw) To: Kévin Le Gouguec Cc: Eli Zaretskii, 68183, Tom Tromey, Sean Whitton, Juri Linkov On 20/08/2024 09:15, Kévin Le Gouguec wrote: > Thanks for weighing in. Since I am not a frequent vc-dir user, my > disposition would then be to avoid committing to sophisticated user > options until there is demand for it. We're in agreement then. > So, at this stage, ISTM: > > * We could apply [patch 1] (new tests) on emacs-30. > > * I should draft a NEWS entry for the new header, for emacs-30. > > * We could apply [patch 2] (refactoring; modulo the considerations > re. vc-git--cmds-in-progress discussed with Sean) on master, if we > feel like it makes vc-git-dir-extra-headers easier to work with. Thanks! NEWS for emacs-30 sounds good, and I've pushed the other two changes (tests and refactoring) to the master branch. ^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#68183: 28.3; vc-dir fails when I have a certain branch checked out 2024-08-21 0:42 ` Dmitry Gutov @ 2024-08-21 1:40 ` Sean Whitton 2024-08-21 7:05 ` Kévin Le Gouguec 0 siblings, 1 reply; 29+ messages in thread From: Sean Whitton @ 2024-08-21 1:40 UTC (permalink / raw) To: Dmitry Gutov Cc: Juri Linkov, Eli Zaretskii, 68183, Tom Tromey, Kévin Le Gouguec Hello, On Wed 21 Aug 2024 at 03:42am +03, Dmitry Gutov wrote: > NEWS for emacs-30 sounds good, and I've pushed the other two changes (tests > and refactoring) to the master branch. Hang on, I think Kevin hadn't incorporated my requested changes into that version :) Kevin, could you provide a follow-up patch, please? Thanks. -- Sean Whitton ^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#68183: 28.3; vc-dir fails when I have a certain branch checked out 2024-08-21 1:40 ` Sean Whitton @ 2024-08-21 7:05 ` Kévin Le Gouguec 2024-08-21 7:59 ` Sean Whitton 0 siblings, 1 reply; 29+ messages in thread From: Kévin Le Gouguec @ 2024-08-21 7:05 UTC (permalink / raw) To: Sean Whitton; +Cc: Dmitry Gutov, Eli Zaretskii, 68183, Tom Tromey, Juri Linkov [-- Attachment #1: Type: text/plain, Size: 692 bytes --] Sean Whitton <spwhitton@spwhitton.name> writes: > Hello, > > On Wed 21 Aug 2024 at 03:42am +03, Dmitry Gutov wrote: > >> NEWS for emacs-30 sounds good, (ACK'ing Eli's request to submit here for review before pushing 👌) >> and I've pushed the other two changes (tests >> and refactoring) to the master branch. (Thanks!) > Hang on, I think Kevin hadn't incorporated my requested changes into > that version :) (Right, I've been less reactive than ideal these past few days, apologies for that) > Kevin, could you provide a follow-up patch, please? Thanks. Something like the attached, right? Tested manually with a rebase. [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: 0001-Restore-vc-git-helper-function-bug-68183.patch --] [-- Type: text/x-diff, Size: 1648 bytes --] From beb4fd8874fb4c03ce2b8b789bcb7a4b7ee411b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Le=20Gouguec?= <kevin.legouguec@gmail.com> Date: Wed, 21 Aug 2024 08:45:00 +0200 Subject: [PATCH] Restore vc-git helper function (bug#68183) * lisp/vc/vc-git.el (vc-git--cmds-in-progress): Restore; it was removed in a previous refactoring patch, but we may still find some use for it. (vc-git-dir--in-progress-headers): Use it. --- lisp/vc/vc-git.el | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/lisp/vc/vc-git.el b/lisp/vc/vc-git.el index 4d631c7e032..dedf6fdd219 100644 --- a/lisp/vc/vc-git.el +++ b/lisp/vc/vc-git.el @@ -748,8 +748,8 @@ or an empty string if none." (and tracking (fmt "Tracking" tracking)) (and remote-url (fmt "Remote" remote-url))))))) -(defun vc-git-dir--in-progress-headers () - "Return headers for Git commands in progress in this worktree." +(defun vc-git--cmds-in-progress () + "Return a list of Git commands in progress in this worktree." (let ((gitdir (vc-git--git-path)) cmds) ;; See contrib/completion/git-prompt.sh in git.git. @@ -765,6 +765,11 @@ or an empty string if none." (push 'merge cmds)) (when (file-exists-p (expand-file-name "BISECT_START" gitdir)) (push 'bisect cmds)) + cmds)) + +(defun vc-git-dir--in-progress-headers () + "Return headers for Git commands in progress in this worktree." + (let ((cmds (vc-git--cmds-in-progress))) (cl-flet ((fmt (cmd name) (when (memq cmd cmds) ;; For now just a heading, key bindings can be added -- 2.39.2 ^ permalink raw reply related [flat|nested] 29+ messages in thread
* bug#68183: 28.3; vc-dir fails when I have a certain branch checked out 2024-08-21 7:05 ` Kévin Le Gouguec @ 2024-08-21 7:59 ` Sean Whitton 2024-08-21 12:29 ` Dmitry Gutov 0 siblings, 1 reply; 29+ messages in thread From: Sean Whitton @ 2024-08-21 7:59 UTC (permalink / raw) To: Kévin Le Gouguec Cc: Dmitry Gutov, Eli Zaretskii, 68183, Tom Tromey, Juri Linkov Hello, On Wed 21 Aug 2024 at 09:05am +02, Kévin Le Gouguec wrote: > (Right, I've been less reactive than ideal these past few days, > apologies for that) No, I think it was that Dmitry misunderstood you and thought that was the final version of the patch. >> Kevin, could you provide a follow-up patch, please? Thanks. > > Something like the attached, right? Tested manually with a rebase. Thanks -- pushed. (I did s/find some use/find use/ in your log message because it seemed more(?) grammatical. Now I read it again, I'm not sure that is true. I hope this is okay with you. It's a very small change.) -- Sean Whitton ^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#68183: 28.3; vc-dir fails when I have a certain branch checked out 2024-08-21 7:59 ` Sean Whitton @ 2024-08-21 12:29 ` Dmitry Gutov 0 siblings, 0 replies; 29+ messages in thread From: Dmitry Gutov @ 2024-08-21 12:29 UTC (permalink / raw) To: Sean Whitton, Kévin Le Gouguec Cc: Eli Zaretskii, 68183, Tom Tromey, Juri Linkov On 21/08/2024 10:59, Sean Whitton wrote: > Hello, > > On Wed 21 Aug 2024 at 09:05am +02, Kévin Le Gouguec wrote: > >> (Right, I've been less reactive than ideal these past few days, >> apologies for that) > No, I think it was that Dmitry misunderstood you and thought that was > the final version of the patch. > >>> Kevin, could you provide a follow-up patch, please? Thanks. >> Something like the attached, right? Tested manually with a rebase. > Thanks -- pushed. > > (I did s/find some use/find use/ in your log message because it seemed > more(?) grammatical. Now I read it again, I'm not sure that is true. > I hope this is okay with you. It's a very small change.) Thank you both. ^ permalink raw reply [flat|nested] 29+ messages in thread
end of thread, other threads:[~2024-08-29 16:41 UTC | newest] Thread overview: 29+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-12-31 18:59 bug#68183: 28.3; vc-dir fails when I have a certain branch checked out Tom Tromey 2023-12-31 19:34 ` Eli Zaretskii 2023-12-31 20:14 ` Tom Tromey 2024-01-03 9:46 ` Kévin Le Gouguec 2024-02-12 8:08 ` Kévin Le Gouguec 2024-02-14 19:56 ` Kévin Le Gouguec 2024-03-13 20:03 ` Kévin Le Gouguec 2024-03-15 2:57 ` Dmitry Gutov 2024-03-16 17:56 ` Kévin Le Gouguec 2024-03-17 1:06 ` Dmitry Gutov 2024-03-17 18:09 ` Kévin Le Gouguec 2024-03-18 15:26 ` Dmitry Gutov 2024-08-07 14:25 ` Kévin Le Gouguec 2024-08-08 0:32 ` Sean Whitton 2024-08-08 7:07 ` Kévin Le Gouguec 2024-08-08 12:02 ` Sean Whitton 2024-08-13 1:32 ` Dmitry Gutov 2024-08-20 6:15 ` Kévin Le Gouguec 2024-08-20 12:15 ` Eli Zaretskii 2024-08-22 7:15 ` Kévin Le Gouguec 2024-08-22 12:46 ` Eli Zaretskii 2024-08-29 15:36 ` Kévin Le Gouguec 2024-08-29 15:46 ` Eli Zaretskii 2024-08-29 16:41 ` Kévin Le Gouguec 2024-08-21 0:42 ` Dmitry Gutov 2024-08-21 1:40 ` Sean Whitton 2024-08-21 7:05 ` Kévin Le Gouguec 2024-08-21 7:59 ` Sean Whitton 2024-08-21 12:29 ` Dmitry Gutov
Code repositories for project(s) associated with this external index https://git.savannah.gnu.org/cgit/emacs.git https://git.savannah.gnu.org/cgit/emacs/org-mode.git This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.