* 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; 12+ 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] 12+ 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; 12+ 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] 12+ 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; 12+ 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] 12+ 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; 12+ 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] 12+ 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; 12+ 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] 12+ 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; 12+ 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] 12+ 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; 12+ 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] 12+ 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; 12+ 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] 12+ 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; 12+ 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] 12+ 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; 12+ 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] 12+ 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; 12+ 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] 12+ 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 0 siblings, 0 replies; 12+ 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] 12+ messages in thread
end of thread, other threads:[~2024-03-18 15:26 UTC | newest] Thread overview: 12+ 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
Code repositories for project(s) associated with this public inbox https://git.savannah.gnu.org/cgit/emacs.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).