Re: [PATCH 3/4] completion: fix branch.autosetup(merge|rebase)

2014-01-03 Thread Junio C Hamano 
Ramkumar Ramachandra artag...@gmail.com writes:

 Junio C Hamano wrote:
 If we are looking at branch.autosetupmerge. followed by something,
 who typed that final dot?

 I admit that it's a very unlikely case. The user did:

   $ branch.autosetupmerTAB

 hit backspace to delete the trailing space, inserted a dot, and hit TAB 
 again.

 If you are working on a topic about
 auto-setup-merge and named your branch autosetupmerge, don't you
 want to be able to configure various aspect of that branch via
 branch.autosetupmerge.{remote,merge} etc., just like you can do so
 for your topic branch via branch.topic.{remote,merge} etc.,
 regardless of your use of autosetupmerge option across branches?

 My reasoning was that being correct was more important that being
 complete. So, if by some horrible chance, the user names her branch
 autosetupmerge, we don't aid her in completions.

 Besides, it smells fishy to me that you need to enumerate and
 special case these two here, and then you have to repeat them below
 in a separate case arm.

 I'm not too irked about correctness in this odd case; seeing that you
 aren't either, I'll resubmit the series without this hunk (+ the hunk
 in remote.pushdefault).

You seem to be calling it incorrect to give the same degree of
completion for a branch the user named autosetupmerge as another
branch topic, but I think it is incorrect not to, so I cannot tell
if we are agreeing or disagreeing.

Puzzled...
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/4] completion: fix branch.autosetup(merge|rebase)

2014-01-03 Thread Ramkumar Ramachandra 
Junio C Hamano wrote:
 You seem to be calling it incorrect to give the same degree of
 completion for a branch the user named autosetupmerge as another
 branch topic, but I think it is incorrect not to, so I cannot tell
 if we are agreeing or disagreeing.

No, what's incorrect is providing completions for

  $ git config branch.autosetupmerge.TAB

when no branch called autosetupmerge exists. The purpose of the hunk
(which I now removed) was to prevent such completions, but it has the
side-effect of also preventing a legitimate completion in the case
when the user really has a branch named autosetupmerge.

What is your take on the issue?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/4] completion: fix branch.autosetup(merge|rebase)

2014-01-03 Thread Junio C Hamano 
Ramkumar Ramachandra artag...@gmail.com writes:

 Junio C Hamano wrote:
 You seem to be calling it incorrect to give the same degree of
 completion for a branch the user named autosetupmerge as another
 branch topic, but I think it is incorrect not to, so I cannot tell
 if we are agreeing or disagreeing.

 No, what's incorrect is providing completions for

   $ git config branch.autosetupmerge.TAB

 when no branch called autosetupmerge exists.  The purpose of the
 hunk (which I now removed) was to prevent such completions, ...

Hmph, but in a repository without 'foo', I just did

$ git config branch.foo.TAB
branch.foo.merge  branch.foo.rebase 
branch.foo.mergeoptions   branch.foo.remote 

and got offered the above. How would that removed hunk that special
cased those autosetupmerge etc. helped such case?

If it _were_ about correctness, and the definition of correctness
were that completing branch.foo.TAB to offer these four variables
is wrong until refs/heads/foo materializes, the fix would have
checked if there already is such a branch and refused to complete
otherwise, not special case a few known names such as autosetup*.

As there is no reason to forbid setting configuration variables for
a branch 'foo' you are going to create before you actually create it
with git branch foo, I do not necessarily agree with the above
definition of correctness, either.

So it was completely bogus hunk and it is good we noticed and
decided to remove it, I guess.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/4] completion: fix branch.autosetup(merge|rebase)

2014-01-02 Thread Junio C Hamano 
Ramkumar Ramachandra artag...@gmail.com writes:

   branch.*.*)
   local pfx=${cur%.*}. cur_=${cur##*.}
 + if [ $pfx == branch.autosetupmerge. ] ||
 + [ $pfx == branch.autosetuprebase. ]; then
 + return
 + fi
   __gitcomp remote pushremote merge mergeoptions rebase $pfx 
 $cur_
   return
   ;;

I do not quite understand this change.

If we are looking at branch.autosetupmerge. followed by something,
who typed that final dot?  If you are working on a topic about
auto-setup-merge and named your branch autosetupmerge, don't you
want to be able to configure various aspect of that branch via
branch.autosetupmerge.{remote,merge} etc., just like you can do so
for your topic branch via branch.topic.{remote,merge} etc.,
regardless of your use of autosetupmerge option across branches?

Besides, it smells fishy to me that you need to enumerate and
special case these two here, and then you have to repeat them below
in a separate case arm.

   branch.*)
   local pfx=${cur%.*}. cur_=${cur#*.}
 - __gitcomp_nl $(__git_heads) $pfx $cur_ .
 + __gitcomp_2 $(__git_heads) 
 + autosetupmerge autosetuprebase
 +  $pfx $cur_ .
   return
   ;;
   guitool.*.*)
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/4] completion: fix branch.autosetup(merge|rebase)

2014-01-02 Thread Ramkumar Ramachandra 
Junio C Hamano wrote:
 If we are looking at branch.autosetupmerge. followed by something,
 who typed that final dot?

I admit that it's a very unlikely case. The user did:

  $ branch.autosetupmerTAB

hit backspace to delete the trailing space, inserted a dot, and hit TAB again.

 If you are working on a topic about
 auto-setup-merge and named your branch autosetupmerge, don't you
 want to be able to configure various aspect of that branch via
 branch.autosetupmerge.{remote,merge} etc., just like you can do so
 for your topic branch via branch.topic.{remote,merge} etc.,
 regardless of your use of autosetupmerge option across branches?

My reasoning was that being correct was more important that being
complete. So, if by some horrible chance, the user names her branch
autosetupmerge, we don't aid her in completions.

 Besides, it smells fishy to me that you need to enumerate and
 special case these two here, and then you have to repeat them below
 in a separate case arm.

I'm not too irked about correctness in this odd case; seeing that you
aren't either, I'll resubmit the series without this hunk (+ the hunk
in remote.pushdefault).
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/4] completion: fix branch.autosetup(merge|rebase)

2013-12-30 Thread Ramkumar Ramachandra 
When attempting to complete

  $ git config branch.autoTAB

'autosetupmerge' and 'autosetuprebase' don't come up. This is because
$cur is matched with branch.* and a list of branches are
completed. Add 'autosetup(merge|rebase)' to the list of branches using
__gitcomp_2 ().

Also take care to not complete

  $ git config branch.autosetupmerge.TAB
  $ git config branch.autosetuprebase.TAB

with the usual branch.name. candidates.

Signed-off-by: Ramkumar Ramachandra artag...@gmail.com
---
 contrib/completion/git-completion.bash | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index 64b20b8..0bda757 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1851,12 +1851,18 @@ _git_config ()
;;
branch.*.*)
local pfx=${cur%.*}. cur_=${cur##*.}
+   if [ $pfx == branch.autosetupmerge. ] ||
+   [ $pfx == branch.autosetuprebase. ]; then
+   return
+   fi
__gitcomp remote pushremote merge mergeoptions rebase $pfx 
$cur_
return
;;
branch.*)
local pfx=${cur%.*}. cur_=${cur#*.}
-   __gitcomp_nl $(__git_heads) $pfx $cur_ .
+   __gitcomp_2 $(__git_heads) 
+   autosetupmerge autosetuprebase
+$pfx $cur_ .
return
;;
guitool.*.*)
-- 
1.8.5.2.227.g53f3478

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html