bug#25710: 25.1.91; vc-retrieve-tag does not offer branch namd completion

bug#25710: 25.1.91; vc-retrieve-tag does not offer branch namd completion

[Tom Tromey]

Today I tried vc-retrieve-tag in a git project.  I was hoping it would
offer branch names for completion, but it did not.

This seems reasonably easy to do as vc-git can already compute the list
of local branch names.

Tom



In GNU Emacs 25.1.91.2 (x86_64-unknown-linux-gnu, GTK+ Version 3.22.7)
 of 2017-02-02 built on bapiya
Repository revision: 7e02a477bbcabb4e65aeecade79b67357c0b9dae
Windowing system distributor 'Fedora Project', version 11.0.11901000
System Description:	Fedora release 25 (Twenty Five)

Configured using:
 'configure --prefix=/home/tromey/Emacs/install/ --with-modules'

Configured features:
XPM JPEG TIFF GIF PNG RSVG IMAGEMAGICK SOUND GPM DBUS GCONF GSETTINGS
NOTIFY LIBSELINUX GNUTLS LIBXML2 FREETYPE M17N_FLT LIBOTF XFT ZLIB
TOOLKIT_SCROLL_BARS GTK3 X11 MODULES

Important settings:
  value of $LANG: en_US.utf8
  value of $XMODIFIERS: @im=ibus
  locale-coding-system: utf-8-unix

Major mode: VC dir

Minor modes in effect:
  vc-parent-buffer: *vc-dir*<trunk>
  shell-dirtrack-mode: t
  diff-auto-refine-mode: t
  which-function-mode: t
  erc-services-mode: t
  erc-list-mode: t
  erc-menu-mode: t
  erc-autojoin-mode: t
  erc-ring-mode: t
  erc-networks-mode: t
  erc-pcomplete-mode: t
  erc-track-mode: t
  erc-match-mode: t
  erc-netsplit-mode: t
  erc-hl-nicks-mode: t
  erc-button-mode: t
  erc-fill-mode: t
  erc-stamp-mode: t
  erc-irccontrols-mode: t
  erc-noncommands-mode: t
  erc-move-to-prompt-mode: t
  erc-readonly-mode: t
  savehist-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  buffer-read-only: t
  column-number-mode: t
  line-number-mode: t
  transient-mark-mode: t

Recent messages:
Saving file /home/tromey/.gitconfig...
Wrote /home/tromey/.gitconfig
user-error: Node has no Next
C-z C-g is undefined
Mark saved where search started
Quit
~/.emacs and /home/tromey/DotEmacs/.emacs are the same file
Quit
C-c C-g is undefined
Mark saved where search started

Load-path shadows:
/home/tromey/.emacs.d/elpa/bubbles-0.5/bubbles hides /home/tromey/Emacs/install/share/emacs/25.1.91/lisp/play/bubbles

Features:
(highlight-refontification tabify man shadow emacsbug conf-mode
two-column iso-transl perl-mode sh-script executable cal-move nnregistry
python tramp-sh tramp tramp-compat tramp-loaddefs trampver ucs-normalize
make-mode etags cursor-sensor mhtml-mode org-bullets org-element
org-rmail org-mhe org-irc org-info org-gnus org-docview doc-view
image-mode org-bibtex bibtex org-bbdb org-w3m org org-macro org-footnote
org-pcomplete org-list org-faces org-entities org-version ob-emacs-lisp
ob ob-tangle ob-ref ob-lob ob-table ob-exp org-src ob-keys ob-comint
ob-core ob-eval org-compat org-macs org-loaddefs gnus-fun gnus-draft
noutline outline ido skeleton css-mode smie eww url-queue trace
find-dired descr-text gdb-shell lisp-mnt url-handlers url-http url-gw
url-auth lex edebug debug m4-mode bbdb-sc supercite regi xref project
log-edit js json map sgml-mode flow-fill gnus-html xml url-cache mm-url
url url-proxy url-privacy url-expand url-methods url-history url-cookie
url-domsuf eieio-opt speedbar sb-image ezimage dframe find-func
whitespace tcl log-view pcvs-util vc-annotate term/xterm xterm
smerge-mode goto-addr shell find-file idutils derived bug-reference
cc-mode cc-fonts cc-guess cc-menus cc-cmds jka-compr mailalias mail-hist
nnir sort gnus-cite smiley shr-color url-util url-parse url-vars shr dom
subr-x browse-url mm-archive gnus-async gnus-bcklg qp gnus-ml disp-table
gnus-topic nndraft nnmh nnfolder utf-7 bbdb-gnus bbdb-mua bbdb-com crm
network-stream nsm starttls gnus-agent gnus-srvr gnus-score score-mode
nnvirtual gnus-msg nntp gnus-cache gnus-registry registry eieio-compat
eieio-base gnus-art mm-uu mml2015 mm-view mml-smime smime dig mailcap
gnus-sum gnus-group gnus-undo smtpmail gnus-start gnus-cloud nnimap
nnmail mail-source tls gnutls utf7 netrc nnoo parse-time gnus-spec
gnus-int gnus-range gnus-win gnus gnus-ems nnheader bbdb-message
sendmail mail-extr message idna dired rfc822 mml mml-sec epg mm-decode
mm-bodies mm-encode mail-parse rfc2231 rfc2047 rfc2045 ietf-drums
mailabbrev mail-utils gmm-utils mailheader gud dwarf-mode copyright
dabbrev misearch multi-isearch vc-mtn vc-hg vc-bzr vc-src vc-sccs vc-svn
vc-cvs vc-rcs add-log rx vc-git diff-mode easy-mmode rust-mode flyspell
ispell diminish edmacro kmacro projectile grep compile ibuf-ext ibuffer
dash appt diary-lib diary-loaddefs cal-menu calendar cal-loaddefs
which-func imenu minimap autorevert filenotify cus-start cus-load status
erc-services erc-list erc-menu erc-join erc-ring erc-networks
erc-pcomplete pcomplete erc-track erc-match erc-netsplit erc-hl-nicks
color erc-button erc-fill erc-stamp wid-edit erc-goodies erc erc-backend
erc-compat format-spec auth-source eieio gnus-util mm-util help-fns
mail-prsvr password-cache thingatpt pp warnings advice vc-dir ewoc vc
vc-dispatcher cc-styles cc-align cc-engine cc-vars cc-defs bbdb
bbdb-site timezone ange-ftp comint ansi-color ring server savehist
finder-inf dwarf-mode-autoloads gdb-shell-autoloads eieio-core
lisppaste-autoloads pydoc-info-autoloads info-look cl-seq cl-macs cl
weblogger-autoloads info package epg-config seq byte-opt gv bytecomp
byte-compile cl-extra help-mode easymenu cconv cl-loaddefs pcase cl-lib
bbdb-loaddefs time-date mule-util tooltip eldoc electric uniquify
ediff-hook vc-hooks lisp-float-type mwheel x-win term/common-win x-dnd
tool-bar dnd fontset image regexp-opt fringe tabulated-list newcomment
elisp-mode lisp-mode prog-mode register page menu-bar rfn-eshadow timer
select scroll-bar mouse jit-lock font-lock syntax facemenu font-core
frame 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 charscript
case-table epa-hook jka-cmpr-hook help simple abbrev minibuffer
cl-preloaded nadvice loaddefs button faces cus-face macroexp files
text-properties overlay sha1 md5 base64 format env code-pages mule
custom widget hashtable-print-readable backquote dbusbind inotify
dynamic-setting system-font-setting font-render-setting move-toolbar gtk
x-toolkit x multi-tty make-network-process emacs)

Memory information:
((conses 16 2705425 530649)
 (symbols 48 139797 578)
 (miscs 40 33520 10538)
 (strings 32 580869 184893)
 (string-bytes 1 16243491)
 (vectors 16 139145)
 (vector-slots 8 2872386 85886)
 (floats 8 936 1276)
 (intervals 56 339189 2699)
 (buffers 976 354))

bug#25710: Acknowledgement (25.1.91; vc-retrieve-tag does not offer branch namd completion)

[Tom Tromey]

This patch seems to work for me.

Tom

commit ead2e9319b5676d2c637474d12d1c4d10fe7167f
Author: Tom Tromey <tom@tromey.com>
Date:   Mon Feb 13 20:11:14 2017 -0700

    Add completion for branch switches in git
    
    Bug#25710
    * lisp/vc/vc-git.el (vc-git-list-branches): New function.
    * lisp/vc/vc.el (vc-retrieve-tag-history): New global.
    (vc-retrieve-tag): Call 'list-branches.  Use vc-retrieve-tag-history.
    (vc-default-list-branches): New function.

diff --git a/lisp/vc/vc-git.el b/lisp/vc/vc-git.el
index 8a22d74..0c11a7f 100644
--- a/lisp/vc/vc-git.el
+++ b/lisp/vc/vc-git.el
@@ -1242,6 +1242,9 @@ vc-git-retrieve-tag
   (let ((default-directory dir))
     (vc-git-command nil 0 nil "checkout" name)))
 
+(defun vc-git-list-branches (dir)
+  (let ((default-directory dir))
+    (vc-git-branches)))
 
 ;;; MISCELLANEOUS
 
diff --git a/lisp/vc/vc.el b/lisp/vc/vc.el
index c66a092..72151c0 100644
--- a/lisp/vc/vc.el
+++ b/lisp/vc/vc.el
@@ -450,6 +450,12 @@
 ;;   does a sanity check whether there aren't any uncommitted changes at
 ;;   or below DIR, and then performs a tree walk, using the `checkout'
 ;;   function to retrieve the corresponding revisions.
+;;
+;; - list-branches (dir)
+;;
+;;   Return a list of strings, each of which names a branch that can
+;;   be switched to with `vc-git-retrieve-tag'.  The default
+;;   implementation returns nil.
 
 ;; MISCELLANEOUS
 ;;
@@ -2151,6 +2157,9 @@ vc-create-tag
   (vc-resynch-buffer dir t t t)
   (message "Making %s... done" (if branchp "branch" "tag")))
 
+(defvar vc-retrieve-tag-history nil
+  "History for `vc-retrieve-tag'.")
+
 ;;;###autoload
 (defun vc-retrieve-tag (dir name)
   "For each file in or below DIR, retrieve their tagged version NAME.
@@ -2172,7 +2181,11 @@ vc-retrieve-tag
 	  ;; to ask for a directory, branches are created at repository level.
 	  default-directory
 	(read-directory-name "Directory: " default-directory default-directory t))
-      (read-string "Tag name to retrieve (default latest revisions): "))))
+      (completing-read "Tag name to retrieve (default latest revisions): "
+                       (vc-call-backend (vc-responsible-backend
+                                         default-directory)
+                                        'list-branches default-directory)
+                       nil nil nil 'vc-retrieve-tag-history))))
   (let ((update (yes-or-no-p "Update any affected buffers? "))
 	(msg (if (or (not name) (string= name ""))
 		 (format "Updating %s... " (abbreviate-file-name dir))
@@ -2853,6 +2866,12 @@ vc-default-retrieve-tag
 		 (vc-call-backend backend 'checkout f nil name)
 		 (when update (vc-resynch-buffer f t t)))))))))
 
+(defun vc-default-list-branches (backend dir)
+  "Default implementation of 'list-branches function.
+This function should return a list of possible branches to switch to.
+The default implementation returns nil."
+  nil)
+
 (defun vc-default-revert (backend file contents-done)
   (unless contents-done
     (let ((rev (vc-working-revision file))

bug#25710: Acknowledgement (25.1.91; vc-retrieve-tag does not offer branch namd completion)

[Dmitry Gutov]

On 14.02.2017 05:13, Tom Tromey wrote:

> +;; - list-branches (dir)

Should we call it list-tags, similarly to the name of the command?

And return tags together with branches in the Git implementation.

bug#25710: Acknowledgement (25.1.91; vc-retrieve-tag does not offer branch namd completion)

[Dmitry Gutov]

On 14.02.2017 23:01, Tom Tromey wrote:

> Dmitry> Should we call it list-tags, similarly to the name of the command?
> 
> This bit is fine by me.

It would be a bit pointless without the second part. If we decide 
against it, list-branches is probably the better choice.

So I wonder what other people think.

> Dmitry> And return tags together with branches in the Git implementation.
> 
> I am not so sure about this.  I've never (literally never) wanted to
> check out a tag with Git.  I do sometimes check out a specific revision.

Me neither, but... people do use tags to mark specific, *important* 
revisions. And supposedly check them out sometimes (maybe to see the 
exact code the "current production" version has?).

I do check out specific revisions sometimes too, but providing 
completion for that is fairly impossible. Completing tags seems like the 
next best thing.

> But... the UI doesn't prohibit this, it just only offers branches for
> completion.  I tend to think that's the most useful.

Branches are most useful indeed.

> Another idea would be to add a defcustom for this.

I don't imagine having tags in there would really hurt, even if we don't 
pick them. IME tags usually follow a particular naming convention, so 
they won't interfere with completion.

bug#25710: Acknowledgement (25.1.91; vc-retrieve-tag does not offer branch namd completion)

[Tom Tromey]

>>>>> "Dmitry" == Dmitry Gutov <dgutov@yandex.ru> writes:

>> Another idea would be to add a defcustom for this.

Dmitry> I don't imagine having tags in there would really hurt, even if we
Dmitry> don't pick them. IME tags usually follow a particular naming
Dmitry> convention, so they won't interfere with completion.

Emacs itself has 144 tags, while I have about 16 branches.
So for me at least, for this project, that's 90% clutter and 10% things
I might actually want to complete.

For my gdb checkout I have 32 branches and there are 636 tags, so the
ratio is even worse there -- about 5% useful things.

Tom

bug#25710: Branch completion on vc-retrive-tag

[Artem Malyshev]

[-- Attachment #1: Type: text/plain, Size: 451 bytes --]

Hello,

Thank you guys for taking this feature into account.

Currently I'm working on Django project. It has 43 branches and 155 tags.
Most of them I don't need of course. Usually I work in 4-5 local branches.
Every thing else have origin prefix and probably outdated from master by
ten years.

I guess that complete on full list of tags plus branches is good to be
similar to vc-revision-other-window. But I don't know until I try.

Regards, Artem.

[-- Attachment #2: Type: text/html, Size: 557 bytes --]

bug#25710: Acknowledgement (25.1.91; vc-retrieve-tag does not offer branch namd completion)

[Dmitry Gutov]

[-- Attachment #1: Type: text/plain, Size: 832 bytes --]

On 15.02.2017 06:35, Tom Tromey wrote:

> Emacs itself has 144 tags, while I have about 16 branches.
> So for me at least, for this project, that's 90% clutter and 10% things
> I might actually want to complete.

All Emacs tags start with "emacs-", so if you just input any other 
character and press TAB, you won't see the tags.

> For my gdb checkout I have 32 branches and there are 636 tags, so the
> ratio is even worse there -- about 5% useful things.

I think we should consider whether it's likely to be a problem, and not 
just reject the unlikely options. Because other people's workflows can 
be different.

And I do anticipate having to check out a particular release version, 
someday.

Further, like Artem reminded us, we already have a revisions completion 
table. So the attached patch seems preferable to me. WDYT?

[-- Attachment #2: vc-retrieve-tag-completion.diff --]
[-- Type: text/x-patch, Size: 1898 bytes --]

diff --git a/lisp/vc/vc.el b/lisp/vc/vc.el
index c308856..358f36a 100644
--- a/lisp/vc/vc.el
+++ b/lisp/vc/vc.el
@@ -2163,18 +2163,22 @@ vc-retrieve-tag
 locked files at or below DIR (but if NAME is empty, locked files are
 allowed and simply skipped)."
   (interactive
-   (let ((granularity
-	  (vc-call-backend (vc-responsible-backend default-directory)
-			   'revision-granularity)))
+   (let* ((granularity
+           (vc-call-backend (vc-responsible-backend default-directory)
+                            'revision-granularity))
+          (dir
+           (if (eq granularity 'repository)
+               ;; For VC's that do not work at file level, it's pointless
+               ;; to ask for a directory, branches are created at repository level.
+               ;; XXX: Either we call expand-file-name here, or use
+               ;; file-in-directory-p inside vc-resynch-buffers-in-directory.
+               (expand-file-name (vc-root-dir))
+             (read-directory-name "Directory: " default-directory nil t))))
      (list
-      (if (eq granularity 'repository)
-	  ;; For VC's that do not work at file level, it's pointless
-	  ;; to ask for a directory, branches are created at repository level.
-          ;; XXX: Either we call expand-file-name here, or use
-          ;; file-in-directory-p inside vc-resynch-buffers-in-directory.
-	  (expand-file-name (vc-root-dir))
-	(read-directory-name "Directory: " default-directory default-directory t))
-      (read-string "Tag name to retrieve (default latest revisions): "))))
+      dir
+      (vc-read-revision "Tag name to retrieve (default latest revisions): "
+                        (list dir)
+                        (vc-responsible-backend dir)))))
   (let ((update (yes-or-no-p "Update any affected buffers? "))
 	(msg (if (or (not name) (string= name ""))
 		 (format "Updating %s... " (abbreviate-file-name dir))

bug#25710: Acknowledgement (25.1.91; vc-retrieve-tag does not offer branch namd completion)

[Tom Tromey]

>>>>> "Dmitry" == Dmitry Gutov <dgutov@yandex.ru> writes:

Dmitry> I think we should consider whether it's likely to be a problem, and
Dmitry> not just reject the unlikely options. Because other people's workflows
Dmitry> can be different.

That seems fair.

I still want to add a defcustom to control this.
For all my cases, the tags would just completely drown out the
usefulness of this feature.  I don't mind writing that separately.

Tom

bug#25710: Acknowledgement (25.1.91; vc-retrieve-tag does not offer branch namd completion)

[Dmitry Gutov]

On 17.02.2017 06:09, Tom Tromey wrote:

> That seems fair.

Thanks, installed.

> I still want to add a defcustom to control this.
> For all my cases, the tags would just completely drown out the
> usefulness of this feature.  I don't mind writing that separately.

Will it change how vc-git-revision-completion-table works, across all 
commands? If so, I'm good with it.

bug#25710: Acknowledgement (25.1.91; vc-retrieve-tag does not offer branch namd completion)

[Tom Tromey]

>>>>> "Dmitry" == Dmitry Gutov <dgutov@yandex.ru> writes:

Dmitry> Will it change how vc-git-revision-completion-table works,
Dmitry> across all commands? If so, I'm good with it.

Yeah.  What do you think of the appended?

Tom

diff --git a/lisp/vc/vc-git.el b/lisp/vc/vc-git.el
index 1a3f1bf..d20b10e 100644
--- a/lisp/vc/vc-git.el
+++ b/lisp/vc/vc-git.el
@@ -180,6 +180,16 @@ vc-git-log-output-coding-system
   :type '(coding-system :tag "Coding system to decode Git log output")
   :version "25.1")
 
+(defcustom vc-git-revision-complete-only-branches nil
+  "Control whether tags are returned by revision completion for Git.
+
+When non-nil, only branches and remotes will be returned by
+`vc-git-revision-completion-table'.  This is used by various VC
+commands when completing branch names.  When nil, tags are also
+included in the completions."
+  :type 'boolean
+  :version "26.1")
+
 ;; History of Git commands.
 (defvar vc-git-history nil)
 
@@ -1187,9 +1197,11 @@ vc-git-revision-table
     (with-temp-buffer
       (vc-git-command t nil nil "for-each-ref" "--format=%(refname)")
       (goto-char (point-min))
-      (while (re-search-forward "^refs/\\(heads\\|tags\\|remotes\\)/\\(.*\\)$"
-                                nil t)
-        (push (match-string 2) table)))
+      (let ((regexp (if vc-git-revision-complete-only-branches
+                        "^refs/\\(heads\\|remotes\\)/\\(.*\\)$"
+                      "^refs/\\(heads\\|tags\\|remotes\\)/\\(.*\\)$")))
+        (while (re-search-forward regexp nil t)
+          (push (match-string 2) table))))
     table))
 
 (defun vc-git-revision-completion-table (files)

bug#25710: Acknowledgement (25.1.91; vc-retrieve-tag does not offer branch namd completion)

[Dmitry Gutov]

On 04.03.2017 20:06, Tom Tromey wrote:
>>>>>> "Dmitry" == Dmitry Gutov <dgutov@yandex.ru> writes:
> 
> Dmitry> Will it change how vc-git-revision-completion-table works,
> Dmitry> across all commands? If so, I'm good with it.
> 
> Yeah.  What do you think of the appended?

Looks good, thanks.

bug#25710: Acknowledgement (25.1.91; vc-retrieve-tag does not offer branch namd completion)

[Stefan Kangas]

Dmitry Gutov <dgutov@yandex.ru> writes:

> On 04.03.2017 20:06, Tom Tromey wrote:
>>>>>>> "Dmitry" == Dmitry Gutov <dgutov@yandex.ru> writes:
>> Dmitry> Will it change how vc-git-revision-completion-table works,
>> Dmitry> across all commands? If so, I'm good with it.
>> Yeah.  What do you think of the appended?
>
> Looks good, thanks.

That was 3 years ago.

It seems like the patch here was never installed.  Should it be?

Best regards,
Stefan Kangas

bug#25710: Acknowledgement (25.1.91; vc-retrieve-tag does not offer branch namd completion)

[Dmitry Gutov]

On 11.08.2020 10:56, Stefan Kangas wrote:
> It seems like the patch here was never installed.  Should it be?

I'm still okay with it.

Kinda curious why Tom checked out of the discussion, since it was 
apparently important for his use cases.

bug#25710: Acknowledgement (25.1.91; vc-retrieve-tag does not offer branch namd completion)

[Lars Ingebrigtsen]

Dmitry Gutov <dgutov@yandex.ru> writes:

> On 11.08.2020 10:56, Stefan Kangas wrote:
>> It seems like the patch here was never installed.  Should it be?
>
> I'm still okay with it.

OK; applied to Emacs 28.

> Kinda curious why Tom checked out of the discussion, since it was
> apparently important for his use cases.

Abducted by aliens?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no