bug#46459: 28.0.50; Shared keymap for Git VC dir "Branch" header?

bug#46459: 28.0.50; Shared keymap for Git VC dir "Branch" header?

[Protesilaos Stavrou]

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

Dear maintainers,

In vc-git.el the header for vc-dir stashes is propertized with its own
keymap.  This means that while point is over the value of the "Stash:"
header, the specified keymap comes into effect, instead of the main one
for vc-dir.  The latter takes effect when point is anywhere but that
header's value.

Should this principle be applied to the value of the "Branch" header as
well?  The attached patch is a proof-of-concept to that end.

If you believe this is worth doing, what else do you think should go
into such a patch from a usability standpoint?

All the best,
Protesilaos

-- 
Protesilaos Stavrou
protesilaos.com

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Add-shared-keymap-to-Git-VC-dir-Branch-header.patch --]
[-- Type: text/x-patch, Size: 1478 bytes --]

From 8945cf63f2fc16881b95ed5be209885b73b9449f Mon Sep 17 00:00:00 2001
Message-Id: <8945cf63f2fc16881b95ed5be209885b73b9449f.1613112064.git.info@protesilaos.com>
From: Protesilaos Stavrou <info@protesilaos.com>
Date: Fri, 12 Feb 2021 08:40:33 +0200
Subject: [PATCH] Add shared keymap to Git VC dir "Branch" header

* lisp/vc/vc-git.el (vc-git-branch-shared-map): Define new keymap.
(vc-git-dir-extra-headers): Propertize header with new keymap.
---
 lisp/vc/vc-git.el | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/lisp/vc/vc-git.el b/lisp/vc/vc-git.el
index 25ae26d746..030af2a379 100644
--- a/lisp/vc/vc-git.el
+++ b/lisp/vc/vc-git.el
@@ -656,6 +656,13 @@ vc-git-dir-status-files
                                  :files files
                                  :update-function update-function)))
 
+(defvar vc-git-branch-shared-map
+  (let ((map (make-sparse-keymap)))
+    (define-key map "c" 'vc-create-tag)
+    (define-key map "l" 'vc-print-branch-log)
+    (define-key map "t" 'vc-retrieve-tag)
+    map))
+
 (defvar vc-git-stash-shared-map
   (let ((map (make-sparse-keymap)))
     (define-key map "S" 'vc-git-stash-snapshot)
@@ -814,7 +821,8 @@ vc-git-dir-extra-headers
     (concat
      (propertize "Branch     : " 'face 'vc-dir-header)
      (propertize branch
-		 'face 'vc-dir-header-value)
+		 'face 'vc-dir-header-value
+                 'keymap vc-git-branch-shared-map)
      (when remote-url
        (concat
 	"\n"
-- 
2.30.0

bug#46459: 28.0.50; Shared keymap for Git VC dir "Branch" header?

[Robert Pluim]

>>>>> On Fri, 12 Feb 2021 08:54:13 +0200, Protesilaos Stavrou <info@protesilaos.com> said:

    Protesilaos> Dear maintainers,
    Protesilaos> In vc-git.el the header for vc-dir stashes is propertized with its own
    Protesilaos> keymap.  This means that while point is over the value of the "Stash:"
    Protesilaos> header, the specified keymap comes into effect, instead of the main one
    Protesilaos> for vc-dir.  The latter takes effect when point is anywhere but that
    Protesilaos> header's value.

    Protesilaos> Should this principle be applied to the value of the "Branch" header as
    Protesilaos> well?  The attached patch is a proof-of-concept to that end.

    Protesilaos> If you believe this is worth doing, what else do you think should go
    Protesilaos> into such a patch from a usability standpoint?

Looks good to me, although Iʼve just noticed the vc and vc-dir have
confusingly different bindings for vc-retrieve-tag and vc-create-tag
Maybe put vc-create-tag on C-x v c as well?

Robert

bug#46459: 28.0.50; Shared keymap for Git VC dir "Branch" header?

[Dmitry Gutov]

On 12.02.2021 11:55, Robert Pluim wrote:
> Looks good to me, although Iʼve just noticed the vc and vc-dir have
> confusingly different bindings for vc-retrieve-tag and vc-create-tag
> Maybe put vc-create-tag on C-x v c as well?

The key binding space is limited, though. Perhaps we should decide on a 
key and use it in both cases.

bug#46459: 28.0.50; Shared keymap for Git VC dir "Branch" header?

[Juri Linkov]

> Looks good to me, although Iʼve just noticed the vc and vc-dir have
> confusingly different bindings for vc-retrieve-tag and vc-create-tag
> Maybe put vc-create-tag on C-x v c as well?

Then the freed key C-x v s could be bound to vc-log-search
(mnemonically more suitable for s = search):

  C-x v c - vc-create-tag
  C-x v s - vc-log-search

bug#46459: 28.0.50; Shared keymap for Git VC dir "Branch" header?

[Dmitry Gutov]

On 13.02.2021 20:20, Juri Linkov wrote:
> Then the freed key C-x v s could be bound to vc-log-search
> (mnemonically more suitable for s = search):
> 
>    C-x v c - vc-create-tag
>    C-x v s - vc-log-search

Good idea, I like that.

bug#46459: 28.0.50; Shared keymap for Git VC dir "Branch" header?

[Robert Pluim]

>>>>> On Sat, 13 Feb 2021 22:05:52 +0200, Dmitry Gutov <dgutov@yandex.ru> said:

    Dmitry> On 13.02.2021 20:20, Juri Linkov wrote:
    >> Then the freed key C-x v s could be bound to vc-log-search
    >> (mnemonically more suitable for s = search):
    >> C-x v c - vc-create-tag
    >> C-x v s - vc-log-search

    Dmitry> Good idea, I like that.

Works for me, but Iʼve used up my "change default behaviour" commit
bit for the week :-)

Although what do we do with vc-retrieve-tag in vc-dir (itʼs on B s)?
Move it to B r to mirror C-x v r?

Robert

bug#46459: 28.0.50; Shared keymap for Git VC dir "Branch" header?

[Dmitry Gutov]

Hi!

On 12.02.2021 08:54, Protesilaos Stavrou wrote:
> In vc-git.el the header for vc-dir stashes is propertized with its own
> keymap.  This means that while point is over the value of the "Stash:"
> header, the specified keymap comes into effect, instead of the main one
> for vc-dir.  The latter takes effect when point is anywhere but that
> header's value.
> 
> Should this principle be applied to the value of the "Branch" header as
> well?  The attached patch is a proof-of-concept to that end.
> 
> If you believe this is worth doing, what else do you think should go
> into such a patch from a usability standpoint?

It seems less obviously useful, given that the stashes list allows one 
to interact with a particular stash (whether it is to view it or delete 
it), whereas this keymap should only save the user one S-b key sequence, 
and only if they figure out to move point to the branch name. I don't 
really object, though, if people like it.

Speaking of usability, I guess something that would make this feature 
more discoverable could help, like a button on top of the branch name, 
and a menu similar to the stashes' one.

bug#46459: 28.0.50; Shared keymap for Git VC dir "Branch" header?

[Protesilaos Stavrou]

On 2021-02-13, 02:50 +0200, Dmitry Gutov <dgutov@yandex.ru> wrote:

> Hi!
>
> On 12.02.2021 08:54, Protesilaos Stavrou wrote:
>> In vc-git.el the header for vc-dir stashes is propertized with its own
>> keymap.  This means that while point is over the value of the "Stash:"
>> header, the specified keymap comes into effect, instead of the main one
>> for vc-dir.  The latter takes effect when point is anywhere but that
>> header's value.
>> Should this principle be applied to the value of the "Branch" header
>> as
>> well?  The attached patch is a proof-of-concept to that end.
>> If you believe this is worth doing, what else do you think should go
>> into such a patch from a usability standpoint?
>
> It seems less obviously useful, given that the stashes list allows one
> to interact with a particular stash (whether it is to view it or delete
> it), whereas this keymap should only save the user one S-b key sequence,
> and only if they figure out to move point to the branch name. I don't
> really object, though, if people like it.
>
> Speaking of usability, I guess something that would make this feature
> more discoverable could help, like a button on top of the branch name,
> and a menu similar to the stashes' one.

You are right: it does not add much value on its own.  A button is the
way to go, perhaps with its list of branches in a contracted state by
default.  That could also be accompanied by some extra branch-related
commands to make the button's list more useful.

At any rate, that is outside the scope of this bug report.  Please feel
free to close it---sorry for the noise.

-- 
Protesilaos Stavrou
protesilaos.com

bug#46459: 28.0.50; Shared keymap for Git VC dir "Branch" header?

[Lars Ingebrigtsen]

Protesilaos Stavrou <info@protesilaos.com> writes:

> At any rate, that is outside the scope of this bug report.  Please feel
> free to close it---sorry for the noise.

I think the conclusion here was that we don't want to add a keymap on
the Branch header, so I'm closing this bug report.

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