Re: [PATCH 3/4] completion: fix branch.autosetup(merge|rebase)
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)
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)
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)
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)
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