[PATCH] git.el: Only show completions from refs/heads, refs/remotes and refs/tags

[PATCH] git.el: Only show completions from refs/heads, refs/remotes and refs/tags

[David Kågedal]

Otherwise it will pick up e.g. lots of irrelevant stuff from
refs/bisect, refs/stash or refs/patches (for StGit users).

Signed-off-by: David Kågedal <davidk@lysator.liu.se>
---
 contrib/emacs/git.el |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/contrib/emacs/git.el b/contrib/emacs/git.el
index feb229c..125e684 100644
--- a/contrib/emacs/git.el
+++ b/contrib/emacs/git.el
@@ -1388,7 +1388,10 @@ With a prefix arg, diff the marked files instead."
 (defun git-read-commit-name (prompt &optional default)
   "Ask for a commit name, with completion for local branch, remote branch and tag."
   (completing-read prompt
-                   (list* "HEAD" "ORIG_HEAD" "FETCH_HEAD" (mapcar #'car (git-for-each-ref)))
+                   (list* "HEAD" "ORIG_HEAD" "FETCH_HEAD"
+                          (mapcar #'car (git-for-each-ref "refs/heads"
+                                                          "refs/remotes"
+                                                          "refs/tags")))
 		   nil nil nil nil default))
 
 (defun git-checkout (branch &optional merge)
-- 
1.6.2.rc1.21.gda6d

Re: [PATCH] git.el: Only show completions from refs/heads, refs/remotes and refs/tags

[David Kågedal]

David Kågedal <davidk@lysator.liu.se> writes:

> Otherwise it will pick up e.g. lots of irrelevant stuff from
> refs/bisect, refs/stash or refs/patches (for StGit users).
>
> Signed-off-by: David Kågedal <davidk@lysator.liu.se>
> ---
>  contrib/emacs/git.el |    5 ++++-
>  1 files changed, 4 insertions(+), 1 deletions(-)
>
> diff --git a/contrib/emacs/git.el b/contrib/emacs/git.el
> index feb229c..125e684 100644
> --- a/contrib/emacs/git.el
> +++ b/contrib/emacs/git.el
> @@ -1388,7 +1388,10 @@ With a prefix arg, diff the marked files instead."
>  (defun git-read-commit-name (prompt &optional default)
>    "Ask for a commit name, with completion for local branch, remote branch and tag."
>    (completing-read prompt
> -                   (list* "HEAD" "ORIG_HEAD" "FETCH_HEAD" (mapcar #'car (git-for-each-ref)))
> +                   (list* "HEAD" "ORIG_HEAD" "FETCH_HEAD"

Sorry, I didn't check this properly. I thought I was fixing inside the
git-checkout function, but this is git-read-commit-name which is used
in more than one place.

But for git-checkout, I would like to see a much shorter list of named
commits, namely those that can be called "branches". I'll come up with
a better patch.

> +                          (mapcar #'car (git-for-each-ref "refs/heads"
> +                                                          "refs/remotes"
> +                                                          "refs/tags")))
>  		   nil nil nil nil default))
>  
>  (defun git-checkout (branch &optional merge)

-- 
David Kågedal

Re: [PATCH] git.el: Only show completions from refs/heads, refs/remotes and refs/tags

[David Kågedal]

David Kågedal <davidk@lysator.liu.se> writes:

> David Kågedal <davidk@lysator.liu.se> writes:
>
>> Otherwise it will pick up e.g. lots of irrelevant stuff from
>> refs/bisect, refs/stash or refs/patches (for StGit users).
>>
>> Signed-off-by: David Kågedal <davidk@lysator.liu.se>
>> ---
>>  contrib/emacs/git.el |    5 ++++-
>>  1 files changed, 4 insertions(+), 1 deletions(-)
>>
>> diff --git a/contrib/emacs/git.el b/contrib/emacs/git.el
>> index feb229c..125e684 100644
>> --- a/contrib/emacs/git.el
>> +++ b/contrib/emacs/git.el
>> @@ -1388,7 +1388,10 @@ With a prefix arg, diff the marked files instead."
>>  (defun git-read-commit-name (prompt &optional default)
>>    "Ask for a commit name, with completion for local branch, remote branch and tag."
>>    (completing-read prompt
>> -                   (list* "HEAD" "ORIG_HEAD" "FETCH_HEAD" (mapcar #'car (git-for-each-ref)))
>> +                   (list* "HEAD" "ORIG_HEAD" "FETCH_HEAD"
>
> Sorry, I didn't check this properly. I thought I was fixing inside the
> git-checkout function, but this is git-read-commit-name which is used
> in more than one place.
>
> But for git-checkout, I would like to see a much shorter list of named
> commits, namely those that can be called "branches".

... which means that I would like git-read-commit-name to actually
work as its documentation string says it does: "Ask for a commit name,
with completion for local branch, remote branch and tag."

-- 
David Kågedal

Re: [PATCH] git.el: Only show completions from refs/heads, refs/remotes and refs/tags

[Alexandre Julliard]

David Kågedal <davidk@lysator.liu.se> writes:

> Sorry, I didn't check this properly. I thought I was fixing inside the
> git-checkout function, but this is git-read-commit-name which is used
> in more than one place.
>
> But for git-checkout, I would like to see a much shorter list of named
> commits, namely those that can be called "branches". I'll come up with
> a better patch.

I think it's reasonable to do this for the other uses of
git-read-commit-name too, so your original patch looks OK.

-- 
Alexandre Julliard
julliard@winehq.org

Re: [PATCH] git.el: Only show completions from refs/heads, refs/remotes and refs/tags

[David Kågedal]

Alexandre Julliard <julliard@winehq.org> writes:

> David Kågedal <davidk@lysator.liu.se> writes:
>
>> Sorry, I didn't check this properly. I thought I was fixing inside the
>> git-checkout function, but this is git-read-commit-name which is used
>> in more than one place.
>>
>> But for git-checkout, I would like to see a much shorter list of named
>> commits, namely those that can be called "branches". I'll come up with
>> a better patch.
>
> I think it's reasonable to do this for the other uses of
> git-read-commit-name too, so your original patch looks OK.

Actually, I'd like to go one step further. I'm only interested in
branches, and only in branches I can commit to. So only asking for
refs/heads seems the best solution.

And the default parameter to git-read-commit-name is never used, so I
removed it.

And completing-read works fine with an alist, so the mapcar isn't
nedded any more.

Here is an updated patch:

From: David Kågedal <davidk@lysator.liu.se>
Subject: [PATCH] git.el: Only show completions from refs/heads

Signed-off-by: David Kågedal <davidk@lysator.liu.se>
---
 contrib/emacs/git.el |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/contrib/emacs/git.el b/contrib/emacs/git.el
index feb229c..d4f8710 100644
--- a/contrib/emacs/git.el
+++ b/contrib/emacs/git.el
@@ -1385,11 +1385,9 @@ With a prefix arg, diff the marked files instead."
         (push (match-string 1) files)))
     files))
 
-(defun git-read-commit-name (prompt &optional default)
+(defun git-read-commit-name (prompt)
   "Ask for a commit name, with completion for local branch, remote branch and tag."
-  (completing-read prompt
-                   (list* "HEAD" "ORIG_HEAD" "FETCH_HEAD" (mapcar #'car (git-for-each-ref)))
-		   nil nil nil nil default))
+  (completing-read prompt (git-for-each-ref "refs/heads")))
 
 (defun git-checkout (branch &optional merge)
   "Checkout a branch, tag, or any commit.
-- 
1.6.2.rc1.21.gda6d



-- 
David Kågedal

Re: [PATCH] git.el: Only show completions from refs/heads, refs/remotes and refs/tags

[Alexandre Julliard]

David Kågedal <davidk@lysator.liu.se> writes:

> Alexandre Julliard <julliard@winehq.org> writes:
>
>> David Kågedal <davidk@lysator.liu.se> writes:
>>
>>> Sorry, I didn't check this properly. I thought I was fixing inside the
>>> git-checkout function, but this is git-read-commit-name which is used
>>> in more than one place.
>>>
>>> But for git-checkout, I would like to see a much shorter list of named
>>> commits, namely those that can be called "branches". I'll come up with
>>> a better patch.
>>
>> I think it's reasonable to do this for the other uses of
>> git-read-commit-name too, so your original patch looks OK.
>
> Actually, I'd like to go one step further. I'm only interested in
> branches, and only in branches I can commit to. So only asking for
> refs/heads seems the best solution.

I think that's going too far. It's useful to be able to checkout a tag,
or to cherry-pick from a remote branch.

-- 
Alexandre Julliard
julliard@winehq.org

Re: [PATCH] git.el: Only show completions from refs/heads, refs/remotes and refs/tags

[David Kågedal]

Alexandre Julliard <julliard@winehq.org> writes:

> David Kågedal <davidk@lysator.liu.se> writes:
>
>> Alexandre Julliard <julliard@winehq.org> writes:
>>
>>> David Kågedal <davidk@lysator.liu.se> writes:
>>>
>>>> Sorry, I didn't check this properly. I thought I was fixing inside the
>>>> git-checkout function, but this is git-read-commit-name which is used
>>>> in more than one place.
>>>>
>>>> But for git-checkout, I would like to see a much shorter list of named
>>>> commits, namely those that can be called "branches". I'll come up with
>>>> a better patch.
>>>
>>> I think it's reasonable to do this for the other uses of
>>> git-read-commit-name too, so your original patch looks OK.
>>
>> Actually, I'd like to go one step further. I'm only interested in
>> branches, and only in branches I can commit to. So only asking for
>> refs/heads seems the best solution.
>
> I think that's going too far. It's useful to be able to checkout a tag,
> or to cherry-pick from a remote branch.

Cherry-pick, yes. And checkout, sometimes. My problem is that I have a
truckload of remote (svn) branches and that means I can't see the
obvious checkout candidates for all the noise.

But add "refs/remotes" and "refs/tags" to the last patch, and maybe we
have a good compromise.

-- 
David Kågedal

Re: [PATCH] git.el: Only show completions from refs/heads, refs/remotes and refs/tags

[Alexandre Julliard]

David Kågedal <davidk@lysator.liu.se> writes:

> Cherry-pick, yes. And checkout, sometimes. My problem is that I have a
> truckload of remote (svn) branches and that means I can't see the
> obvious checkout candidates for all the noise.
>
> But add "refs/remotes" and "refs/tags" to the last patch, and maybe we
> have a good compromise.

We could of course make it customizable...

-- 
Alexandre Julliard
julliard@winehq.org

[PATCH] git.el: Only show completions from refs/heads

[David Kågedal]

Signed-off-by: David Kågedal <davidk@lysator.liu.se>
---

Here is a version that can to both commit name lookup and branch name
lookup, and with a configuration parameter that determines how to find
branch names to complete on.

contrib/emacs/git.el |   34 +++++++++++++++++++++++++---------
 1 files changed, 25 insertions(+), 9 deletions(-)

diff --git a/contrib/emacs/git.el b/contrib/emacs/git.el
index feb229c..a5138d7 100644
--- a/contrib/emacs/git.el
+++ b/contrib/emacs/git.el
@@ -118,6 +118,12 @@ if there is already one that displays the same directory."
   :group 'git
   :type 'boolean)
 
+(defcustom git-complete-branch-patterns
+  '("refs/heads" "refs/tags" "refs/remotes")
+  "Which patterns to use when completing branch names."
+  :group 'git
+  :type '(repeat string))
+
 
 (defface git-status-face
   '((((class color) (background light)) (:foreground "purple"))
@@ -1385,17 +1391,27 @@ With a prefix arg, diff the marked files instead."
         (push (match-string 1) files)))
     files))
 
-(defun git-read-commit-name (prompt &optional default)
-  "Ask for a commit name, with completion for local branch, remote branch and tag."
-  (completing-read prompt
-                   (list* "HEAD" "ORIG_HEAD" "FETCH_HEAD" (mapcar #'car (git-for-each-ref)))
-		   nil nil nil nil default))
+(defun git-read-commit-name (prompt specials &optional ref-patterns)
+  "Ask for a commit name, with completion.
+If SPECIALS is non-nil, add HEAD and similar names to the list of possible
+completions. The patterns in REF-PATTERNS are passed to `git-for-each-ref'
+to get a list of completions."
+  (let ((refs (apply #'git-for-each-ref ref-patterns)))
+    (completing-read prompt (if specials (list* '("HEAD" . nil)
+                                                '("FETCH_HEAD" . nil)
+                                                '("ORIG_HEAD" . nil)
+                                                refs)
+                              refs))))
+
+(defun git-read-branch-name (prompt)
+  "Ask for a branch name, with completion."
+  (git-read-commit-name prompt nil git-complete-branch-patterns))
 
 (defun git-checkout (branch &optional merge)
   "Checkout a branch, tag, or any commit.
 Use a prefix arg if git should merge while checking out."
   (interactive
-   (list (git-read-commit-name "Checkout: ")
+   (list (git-read-branch-name "Checkout: ")
          current-prefix-arg))
   (unless git-status (error "Not in git-status buffer."))
   (let ((args (list branch "--")))
@@ -1405,7 +1421,7 @@ Use a prefix arg if git should merge while checking out."
 
 (defun git-branch (branch)
   "Create a branch from the current HEAD and switch to it."
-  (interactive (list (git-read-commit-name "Branch: ")))
+  (interactive (list (git-read-branch-name "Branch: ")))
   (unless git-status (error "Not in git-status buffer."))
   (if (git-rev-parse (concat "refs/heads/" branch))
       (if (yes-or-no-p (format "Branch %s already exists, replace it? " branch))
@@ -1433,7 +1449,7 @@ amended version of it."
 
 (defun git-cherry-pick-commit (arg)
   "Cherry-pick a commit."
-  (interactive (list (git-read-commit-name "Cherry-pick commit: ")))
+  (interactive (list (git-read-commit-name "Cherry-pick commit: " t)))
   (unless git-status (error "Not in git-status buffer."))
   (let ((commit (git-rev-parse (concat arg "^0"))))
     (unless commit (error "Not a valid commit '%s'." arg))
@@ -1452,7 +1468,7 @@ amended version of it."
 
 (defun git-revert-commit (arg)
   "Revert a commit."
-  (interactive (list (git-read-commit-name "Revert commit: ")))
+  (interactive (list (git-read-commit-name "Revert commit: " t)))
   (unless git-status (error "Not in git-status buffer."))
   (let ((commit (git-rev-parse (concat arg "^0"))))
     (unless commit (error "Not a valid commit '%s'." arg))
-- 
1.6.2.rc1.21.gda6d


-- 
David Kågedal

Re: [PATCH] git.el: Only show completions from refs/heads

[Alexandre Julliard]

David Kågedal <davidk@lysator.liu.se> writes:

> Signed-off-by: David Kågedal <davidk@lysator.liu.se>
> ---
>
> Here is a version that can to both commit name lookup and branch name
> lookup, and with a configuration parameter that determines how to find
> branch names to complete on.

Why do you introduce a separate function for checkouts?  I don't see
much need to have a different set of completions for checkout
vs. cherry-pick, they both can be used with an arbitrary commit anyway.

-- 
Alexandre Julliard
julliard@winehq.org

Re: [PATCH] git.el: Only show completions from refs/heads

[David Kågedal]

Alexandre Julliard <julliard@winehq.org> writes:

> David Kågedal <davidk@lysator.liu.se> writes:
>
>> Signed-off-by: David Kågedal <davidk@lysator.liu.se>
>> ---
>>
>> Here is a version that can to both commit name lookup and branch name
>> lookup, and with a configuration parameter that determines how to find
>> branch names to complete on.
>
> Why do you introduce a separate function for checkouts?  I don't see
> much need to have a different set of completions for checkout
> vs. cherry-pick, they both can be used with an arbitrary commit anyway.

Yes, but the typical uses differ a lot. The (overwhelmingly) typical
use of checkout is to switch to another branch. The typical use of
cherry-pick is to pick any commit and not treat branch heads
specially.

So when switching branches, I obviously want a simple way to select
which branch to switch to. When cherry-picking, I would need some
simple way of picking any single commit, but that's hard so making it
easy to pick any named commit is probably the reasonable solution.

-- 
David Kågedal

Re: [PATCH] git.el: Only show completions from refs/heads

[Alexandre Julliard]

David Kågedal <davidk@lysator.liu.se> writes:

> Yes, but the typical uses differ a lot. The (overwhelmingly) typical
> use of checkout is to switch to another branch. The typical use of
> cherry-pick is to pick any commit and not treat branch heads
> specially.
>
> So when switching branches, I obviously want a simple way to select
> which branch to switch to. When cherry-picking, I would need some
> simple way of picking any single commit, but that's hard so making it
> easy to pick any named commit is probably the reasonable solution.

It sounds like what you want is to customize it independently for each
operation. That would be better than having two completion functions,
where it's not clear how they differ and why only one can be
customized.

-- 
Alexandre Julliard
julliard@winehq.org