Re: [PATCH 3/4] completion: add completer for rev-parse

2013-07-03 Thread Ramkumar Ramachandra
Junio C Hamano wrote:
 For --short and --symbolic-full-name, I have a feeling that we
 should make describe a front-end for these.
 [...]

All very good.  If and when someone does write that functionality (I
personally find the task boring), we can remove this completer.  Until
such a time, the completer is useful and should be merged: so, could
you queue this series if there are no objections?
--
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: add completer for rev-parse

2013-06-30 Thread SZEDER Gábor
On Fri, Jun 28, 2013 at 07:48:07PM +0530, Ramkumar Ramachandra wrote:
 Signed-off-by: Ramkumar Ramachandra artag...@gmail.com
 ---
  contrib/completion/git-completion.bash | 14 ++
  1 file changed, 14 insertions(+)
 
 diff --git a/contrib/completion/git-completion.bash 
 b/contrib/completion/git-completion.bash
 index 278018f..f2959a7 100644
 --- a/contrib/completion/git-completion.bash
 +++ b/contrib/completion/git-completion.bash
 @@ -2247,6 +2247,20 @@ _git_reset ()
   __gitcomp_nl $(__git_refs)
  }
  
 +_git_rev_parse ()
 +{
 + case $cur in
 + --*)
 + __gitcomp 
 + --short --show-toplevel --is-inside-work-tree
 + --symbolic-full-name --verify
 + 

In the completion script we support porcelain commands.  I'm not sure
about 'git rev-parse', but I think it's more plumbing than porcelain.
However, I think the same about 'git ls-tree' and 'git reflog', too,
yet we have support for them in the completion script.

Either way, why these five options?  'git rev-parse' has a lot more
options than that.


 +   return
 +   ;;
 + esac
 + __gitcomp_nl $(__git_refs)
 +}
 +
  _git_revert ()
  {
   case $cur in
 -- 
 1.8.3.1.585.g9832cb9
 
--
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: add completer for rev-parse

2013-06-30 Thread Ramkumar Ramachandra
SZEDER Gábor wrote:
 Either way, why these five options?  'git rev-parse' has a lot more
 options than that.

We have to start somewhere, so I put in the options that I personally use.
--
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: add completer for rev-parse

2013-06-30 Thread Junio C Hamano
SZEDER Gábor sze...@ira.uka.de writes:

 On Fri, Jun 28, 2013 at 07:48:07PM +0530, Ramkumar Ramachandra wrote:
 Signed-off-by: Ramkumar Ramachandra artag...@gmail.com
 ---
  contrib/completion/git-completion.bash | 14 ++
  1 file changed, 14 insertions(+)
 
 diff --git a/contrib/completion/git-completion.bash 
 b/contrib/completion/git-completion.bash
 index 278018f..f2959a7 100644
 --- a/contrib/completion/git-completion.bash
 +++ b/contrib/completion/git-completion.bash
 @@ -2247,6 +2247,20 @@ _git_reset ()
  __gitcomp_nl $(__git_refs)
  }
  
 +_git_rev_parse ()
 +{
 +case $cur in
 +--*)
 +__gitcomp 
 +--short --show-toplevel --is-inside-work-tree
 +--symbolic-full-name --verify
 +

 In the completion script we support porcelain commands.  I'm not sure
 about 'git rev-parse', but I think it's more plumbing than porcelain.
 However, I think the same about 'git ls-tree' and 'git reflog', too,
 yet we have support for them in the completion script.

 Either way, why these five options?  'git rev-parse' has a lot more
 options than that.

I think most of the options not listed here are indeed very low
level plumbings that end users have no reason to type on the command
line, except when they are learning to see how they would use it in
their scripts, of course.

Adding a select few that current Git gives no other easy way to
achieve what the users may want to do is a good short-to-medium term
compromise for usability, and I think the above is a good starting
point.

But our goal should be _not_ to grow that set, but to shrink them by
making rev-parse less end-user facing.

A longer term direction should be to make sure that we do _not_ need
to run rev-parse from the command line by giving usability updates
to other commands and shell helpers, though.

For example, some subcommands of git submodule always required you
to run from the top-level, so you needed some way to find out where
the top level was, but the need to find the top-level is _not_ the
ultimate end-user _want_.  There was no other easy way to achieve
what the users wanted to do (i.e. run git submodule foo command)
without first finding out where the top-level is and to go there
before running it.  The user did not necessarily want to go there,
and giving an easy way to find the top may merely be a workaround.

The true solution for that particular issue may be to teach that
subcommand of git submodule to run from anywhere, which I think
has happened recently.

Another example on the same option.  Nobody should have to type

$ cd $(git rev-parse --show-toplevel)

on the command line, even if there is a legitimate reason why it is
necessary to go to the top.  If it is common enough, just like we
ship completion and prompt in contrib/, we should ship a set of
common shell functions and aliases to let you do the above with:

$ cdtop

which may be defined to be something like [*1*].

And these are illustrations of how we can lose needs to use that
option from the command line.  We should continue to go in that
direction.

For --short and --symbolic-full-name, I have a feeling that we
should make describe a front-end for these.

Just like describe already acts as a front-end for name-rev and
behaves differently, we treat the command as a way to transform an
object name into another form, and in addition to the current do so
by expressing the given rev relative to an appropriate anchor point
mode, teach it more ways to represent the given rev by doing
something different (e.g. these two are about expressing them as
themselves and not as relative to something else, so the describe
command in that mode would not walk the history to find nearby anchor
points).

As to completing --verify, I can see how it may be useful for
people who interactively play with the command examples they find in
scripts, but otherwise I do not see much value for real end-users.

rev-parse foobar would say foobar if that does not refer to an
object and end users are more intelligent than a script to see the
difference without seeing the value of $? and error message when
running on the command line.


[Footnote]

*1* This is typed in my MUA for illustration purposes only, not
tested:

cdtop () {
local eh
eh=$(git rev-parse --is-inside-work-tree) || return
case $eh in
true)
eh=$(git rev-parse --show-toplevel)
test -z $eh || cd $eh
;;
false)
eh=$(git rev-parse --git-dir) || return
cd $eh || return
eh=$(git rev-parse --is-bare-repository) || return
test z$eh = ztrue || cd ..
;;
esac
}
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message 

[PATCH 3/4] completion: add completer for rev-parse

2013-06-28 Thread Ramkumar Ramachandra
Signed-off-by: Ramkumar Ramachandra artag...@gmail.com
---
 contrib/completion/git-completion.bash | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index 278018f..f2959a7 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2247,6 +2247,20 @@ _git_reset ()
__gitcomp_nl $(__git_refs)
 }
 
+_git_rev_parse ()
+{
+   case $cur in
+   --*)
+   __gitcomp 
+   --short --show-toplevel --is-inside-work-tree
+   --symbolic-full-name --verify
+   
+   return
+   ;;
+   esac
+   __gitcomp_nl $(__git_refs)
+}
+
 _git_revert ()
 {
case $cur in
-- 
1.8.3.1.585.g9832cb9

--
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