* 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 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
* 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
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.