[PATCH] submodule: make 'show' an alias for 'summary'

[PATCH] submodule: make 'show' an alias for 'summary'

[Ramkumar Ramachandra]

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 Like 'git remote show', 'git stash show'.

 Documentation/git-submodule.txt |    3 ++-
 git-submodule.sh                |    9 ++++++++-
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index fbbbcb2..498bb5c 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -15,7 +15,7 @@ SYNOPSIS
 'git submodule' [--quiet] init [--] [<path>...]
 'git submodule' [--quiet] update [--init] [-N|--no-fetch] [--rebase]
 	      [--reference <repository>] [--merge] [--recursive] [--] [<path>...]
-'git submodule' [--quiet] summary [--cached|--files] [(-n|--summary-limit) <n>]
+'git submodule' [--quiet] summary|show [--cached|--files] [(-n|--summary-limit) <n>]
 	      [commit] [--] [<path>...]
 'git submodule' [--quiet] foreach [--recursive] <command>
 'git submodule' [--quiet] sync [--] [<path>...]
@@ -151,6 +151,7 @@ If `--recursive` is specified, this command will recurse into the
 registered submodules, and update any nested submodules within.
 
 summary::
+show::
 	Show commit summary between the given commit (defaults to HEAD) and
 	working tree/index. For a submodule in question, a series of commits
 	in the submodule between the given super project commit and the
diff --git a/git-submodule.sh b/git-submodule.sh
index 9210f3a..1d61ebd 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -889,6 +889,13 @@ cmd_summary() {
 	fi
 }
 #
+# Alias for cmd_summary
+#
+cmd_show()
+{
+    cmd_summary "$@"
+}
+#
 # List all submodules, prefixed with:
 #  - submodule not initialized
 #  + different revision checked out
@@ -1049,7 +1056,7 @@ cmd_sync()
 while test $# != 0 && test -z "$command"
 do
 	case "$1" in
-	add | foreach | init | update | status | summary | sync)
+	add | foreach | init | update | status | summary | show | sync)
 		command=$1
 		;;
 	-q|--quiet)
-- 
1.7.10.4

[PATCH] submodule: use abbreviated sha1 in 'status' output

[Ramkumar Ramachandra]

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 git-submodule.sh |   15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/git-submodule.sh b/git-submodule.sh
index 1d61ebd..f8efc52 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -942,27 +942,28 @@ cmd_status()
 		name=$(module_name "$sm_path") || exit
 		url=$(git config submodule."$name".url)
 		displaypath="$prefix$sm_path"
+		sha1_abbr=$(git rev-parse --short "$sha1")
 		if test "$stage" = U
 		then
-			say "U$sha1 $displaypath"
+			say "U$sha1_abbr $displaypath"
 			continue
 		fi
 		if test -z "$url" || ! test -d "$sm_path"/.git -o -f "$sm_path"/.git
 		then
-			say "-$sha1 $displaypath"
+			say "-$sha1_abbr $displaypath"
 			continue;
 		fi
-		set_name_rev "$sm_path" "$sha1"
+		set_name_rev "$sm_path" "$sha1_abbr"
 		if git diff-files --ignore-submodules=dirty --quiet -- "$sm_path"
 		then
-			say " $sha1 $displaypath$revname"
+			say " $sha1_abbr $displaypath$revname"
 		else
 			if test -z "$cached"
 			then
-				sha1=$(clear_local_git_env; cd "$sm_path" && git rev-parse --verify HEAD)
-				set_name_rev "$sm_path" "$sha1"
+				sha1_abbr=$(clear_local_git_env; cd "$sm_path" && git rev-parse --short --verify HEAD)
+				set_name_rev "$sm_path" "$sha1_abbr"
 			fi
-			say "+$sha1 $displaypath$revname"
+			say "+$sha1_abbr $displaypath$revname"
 		fi
 
 		if test -n "$recursive"
-- 
1.7.10.4

Re: [PATCH] submodule: use abbreviated sha1 in 'status' output

[Jens Lehmann]

I'm not against the change per se, but do we really want to risk breaking
scripts which parse the output of "git submodule status" without even
providing a commit message explaining why we did that?

Am 29.09.2012 15:43, schrieb Ramkumar Ramachandra:
> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
> ---
>  git-submodule.sh |   15 ++++++++-------
>  1 file changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/git-submodule.sh b/git-submodule.sh
> index 1d61ebd..f8efc52 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -942,27 +942,28 @@ cmd_status()
>  		name=$(module_name "$sm_path") || exit
>  		url=$(git config submodule."$name".url)
>  		displaypath="$prefix$sm_path"
> +		sha1_abbr=$(git rev-parse --short "$sha1")
>  		if test "$stage" = U
>  		then
> -			say "U$sha1 $displaypath"
> +			say "U$sha1_abbr $displaypath"
>  			continue
>  		fi
>  		if test -z "$url" || ! test -d "$sm_path"/.git -o -f "$sm_path"/.git
>  		then
> -			say "-$sha1 $displaypath"
> +			say "-$sha1_abbr $displaypath"
>  			continue;
>  		fi
> -		set_name_rev "$sm_path" "$sha1"
> +		set_name_rev "$sm_path" "$sha1_abbr"
>  		if git diff-files --ignore-submodules=dirty --quiet -- "$sm_path"
>  		then
> -			say " $sha1 $displaypath$revname"
> +			say " $sha1_abbr $displaypath$revname"
>  		else
>  			if test -z "$cached"
>  			then
> -				sha1=$(clear_local_git_env; cd "$sm_path" && git rev-parse --verify HEAD)
> -				set_name_rev "$sm_path" "$sha1"
> +				sha1_abbr=$(clear_local_git_env; cd "$sm_path" && git rev-parse --short --verify HEAD)
> +				set_name_rev "$sm_path" "$sha1_abbr"
>  			fi
> -			say "+$sha1 $displaypath$revname"
> +			say "+$sha1_abbr $displaypath$revname"
>  		fi
>  
>  		if test -n "$recursive"
> 

Re: [PATCH] submodule: use abbreviated sha1 in 'status' output

[Ramkumar Ramachandra]

Jens Lehmann wrote:
> I'm not against the change per se, but do we really want to risk breaking
> scripts which parse the output of "git submodule status" without even
> providing a commit message explaining why we did that?

Oh, I didn't realize that there might be such scripts.  What
justification do I give in the commit message apart from
prettification?

Ram

Re: [PATCH] submodule: use abbreviated sha1 in 'status' output

[Jens Lehmann]

Am 29.09.2012 16:45, schrieb Ramkumar Ramachandra:
> Jens Lehmann wrote:
>> I'm not against the change per se, but do we really want to risk breaking
>> scripts which parse the output of "git submodule status" without even
>> providing a commit message explaining why we did that?
> 
> Oh, I didn't realize that there might be such scripts.  What
> justification do I give in the commit message apart from
> prettification?

Is a prettification justification enough to break backwards compatibility
and to risk breaking scripts and user expectations? I think we must have
a really good reasons to do that, and just making stuff prettier doesn't
count for me unless we have a strong user demand for that.

I suspect you got the idea for this patch from Marc's recent comment:

Am 24.09.2012 17:07, schrieb Marc Branchaud:
> (Honestly, submodule's status sub-command has always felt more like plumbing
> to me than something a user would work with directly.  Maybe it's just the
> full-length SHA's that put me off...)

That is just a single user so far indicating your patch /could/ be an
improvement. I think we need quite some more votes on that before we
should do a change like this.

Re: [PATCH] submodule: use abbreviated sha1 in 'status' output

[Ramkumar Ramachandra]

Jens Lehmann wrote:
> I suspect you got the idea for this patch from Marc's recent comment:
> [...]

Yes, I did.

> That is just a single user so far indicating your patch /could/ be an
> improvement. I think we need quite some more votes on that before we
> should do a change like this.

I thought it's a porcelain command like 'git status'- we don't need
votes to change the output format of 'git status', do we?

Ram

Re: [PATCH] submodule: use abbreviated sha1 in 'status' output

[Jens Lehmann]

Am 01.10.2012 08:42, schrieb Ramkumar Ramachandra:
> Jens Lehmann wrote:
>> That is just a single user so far indicating your patch /could/ be an
>> improvement. I think we need quite some more votes on that before we
>> should do a change like this.
> 
> I thought it's a porcelain command like 'git status'- we don't need
> votes to change the output format of 'git status', do we?

Sure, we are free to change porcelain output. But that doesn't mean
we shouldn't have a good reason to do so, no? And I'm not sure yet
that it's worth risking to break scripts and user expectations.

And Marc's comment also implies that the status command might not
be heavily used (surely I never use it), so maybe we are just
wasting our time here trying to improve it. And if real users show
interest in shortening the hashes, I won't object that change.

Re: [PATCH] submodule: make 'show' an alias for 'summary'

[Jens Lehmann]

Am 29.09.2012 15:43, schrieb Ramkumar Ramachandra:
> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
> ---
>  Like 'git remote show', 'git stash show'.

I understand the analogy to "git stash show" (as that also displays
a diff similar to what " git submodule summary" does). But "git
remote show" just displays a list of configured remotes, which is
something different. And the other command with a "show" option is
"git notes show", which causes it to display a note. So I think we
are pretty inconsistent here and I see no advantage of adding such
an alias for "git submodule summary". What am I missing?

Re: [PATCH] submodule: make 'show' an alias for 'summary'

[Ramkumar Ramachandra]

Hi Jens,

Jens Lehmann wrote:
> Am 29.09.2012 15:43, schrieb Ramkumar Ramachandra:
>> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
>> ---
>>  Like 'git remote show', 'git stash show'.
>
> I understand the analogy to "git stash show" (as that also displays
> a diff similar to what " git submodule summary" does). But "git
> remote show" just displays a list of configured remotes, which is
> something different. And the other command with a "show" option is
> "git notes show", which causes it to display a note. So I think we
> are pretty inconsistent here and I see no advantage of adding such
> an alias for "git submodule summary". What am I missing?

I used submodules for the first time, and expected 'git submodule
show' to work.  It may not be 100% consistent with the other commands,
but I think the 'summary' is a good match.  Either way, I don't feel
strongly about the patch, so feel free to drop it if you think it's
inappropriate.

Ram

Re: [PATCH] submodule: make 'show' an alias for 'summary'

[Jens Lehmann]

Am 29.09.2012 17:07, schrieb Ramkumar Ramachandra:
> Jens Lehmann wrote:
>> Am 29.09.2012 15:43, schrieb Ramkumar Ramachandra:
>>> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
>>> ---
>>>  Like 'git remote show', 'git stash show'.
>>
>> I understand the analogy to "git stash show" (as that also displays
>> a diff similar to what " git submodule summary" does). But "git
>> remote show" just displays a list of configured remotes, which is
>> something different. And the other command with a "show" option is
>> "git notes show", which causes it to display a note. So I think we
>> are pretty inconsistent here and I see no advantage of adding such
>> an alias for "git submodule summary". What am I missing?
> 
> I used submodules for the first time, and expected 'git submodule
> show' to work.  It may not be 100% consistent with the other commands,
> but I think the 'summary' is a good match.  Either way, I don't feel
> strongly about the patch, so feel free to drop it if you think it's
> inappropriate.

I'm very interested in your feedback as a first time submodule user,
what you wrote above makes sense and explains why you did that patch
(and it would have been nice to read some of it in the commit message
;-). What information did you expect to get from a "git submodule
show" which isn't already provided by "git status" and "git diff"
(especially as they give you some information the "git submodule"
commands don't)?

Re: [PATCH] submodule: make 'show' an alias for 'summary'

[Ramkumar Ramachandra]

Hi Jens,

Jens Lehmann wrote:
> I'm very interested in your feedback as a first time submodule user,
> what you wrote above makes sense and explains why you did that patch
> (and it would have been nice to read some of it in the commit message
> ;-). What information did you expect to get from a "git submodule
> show" which isn't already provided by "git status" and "git diff"
> (especially as they give you some information the "git submodule"
> commands don't)?

I expected 'git submodule show' to list all the submodules, and show
changes to specific submodules like the 'git submodule summary'
output.

Ram

Re: [PATCH] submodule: make 'show' an alias for 'summary'

[Jens Lehmann]

Am 01.10.2012 08:45, schrieb Ramkumar Ramachandra:
> Jens Lehmann wrote:
>> I'm very interested in your feedback as a first time submodule user,
>> what you wrote above makes sense and explains why you did that patch
>> (and it would have been nice to read some of it in the commit message
>> ;-). What information did you expect to get from a "git submodule
>> show" which isn't already provided by "git status" and "git diff"
>> (especially as they give you some information the "git submodule"
>> commands don't)?
> 
> I expected 'git submodule show' to list all the submodules, and show
> changes to specific submodules like the 'git submodule summary'
> output.

Ok, but "git submodule summary" doesn't list all the submodules,
only those with changes. Maybe "git submodule status" is closer
to what you expect (except for the missing changes)?

And - apart from the list of all submodules - the changes to them
are given by "git status" and "git diff --submodule" too, right?
(sometimes I forget that "--submodule" is not enabled by default,
as I'm a heavy "git gui" user, and that option is used there. We
might need a config option to turn that on) Or is there something
you are missing from their output?

Me too would expect a show command to show me a list of all the
submodules and maybe some extra information (is it populated or
not, does it have its .git directory embedded, does it contain
changes). So maybe "show" should be a slightly pimped "status"?

Re: [PATCH] submodule: make 'show' an alias for 'summary'

[Ramkumar Ramachandra]

Hi Jens,

Jens Lehmann wrote:
> Am 01.10.2012 08:45, schrieb Ramkumar Ramachandra:
>> Jens Lehmann wrote:
>>> I'm very interested in your feedback as a first time submodule user,
>>> what you wrote above makes sense and explains why you did that patch
>>> (and it would have been nice to read some of it in the commit message
>>> ;-). What information did you expect to get from a "git submodule
>>> show" which isn't already provided by "git status" and "git diff"
>>> (especially as they give you some information the "git submodule"
>>> commands don't)?
>>
>> I expected 'git submodule show' to list all the submodules, and show
>> changes to specific submodules like the 'git submodule summary'
>> output.
>
> Ok, but "git submodule summary" doesn't list all the submodules,
> only those with changes. Maybe "git submodule status" is closer
> to what you expect (except for the missing changes)?

Yes, "git submodule status" with "git submodule summary"-style change output.

> And - apart from the list of all submodules - the changes to them
> are given by "git status" and "git diff --submodule" too, right?

Oh, I didn't know about "git diff --submodule" at all.

> (sometimes I forget that "--submodule" is not enabled by default,
> as I'm a heavy "git gui" user, and that option is used there. We
> might need a config option to turn that on)

That's a good idea.  I'll write a patch tomorrow.

> Me too would expect a show command to show me a list of all the
> submodules and maybe some extra information (is it populated or
> not, does it have its .git directory embedded, does it contain
> changes). So maybe "show" should be a slightly pimped "status"?

Sure.  Do we want to create a new subcommand though?  Aren't "status"
and "summary" enough already?

Ram

Re: [PATCH] submodule: make 'show' an alias for 'summary'

[Jens Lehmann]

Am 01.10.2012 19:33, schrieb Ramkumar Ramachandra:
> Jens Lehmann wrote:
>> Am 01.10.2012 08:45, schrieb Ramkumar Ramachandra:
>>> Jens Lehmann wrote:
>>>> I'm very interested in your feedback as a first time submodule user,
>>>> what you wrote above makes sense and explains why you did that patch
>>>> (and it would have been nice to read some of it in the commit message
>>>> ;-). What information did you expect to get from a "git submodule
>>>> show" which isn't already provided by "git status" and "git diff"
>>>> (especially as they give you some information the "git submodule"
>>>> commands don't)?
>>>
>>> I expected 'git submodule show' to list all the submodules, and show
>>> changes to specific submodules like the 'git submodule summary'
>>> output.
>>
>> Ok, but "git submodule summary" doesn't list all the submodules,
>> only those with changes. Maybe "git submodule status" is closer
>> to what you expect (except for the missing changes)?
> 
> Yes, "git submodule status" with "git submodule summary"-style change output.

So just aliasing "show" to "summary" would not have done the trick,
right? ;-)

>> And - apart from the list of all submodules - the changes to them
>> are given by "git status" and "git diff --submodule" too, right?
> 
> Oh, I didn't know about "git diff --submodule" at all.
> 
>> (sometimes I forget that "--submodule" is not enabled by default,
>> as I'm a heavy "git gui" user, and that option is used there. We
>> might need a config option to turn that on)
> 
> That's a good idea.  I'll write a patch tomorrow.

Cool! I suspect showing the difference in shortlog style is much
more useful than seeing the hashes.

>> Me too would expect a show command to show me a list of all the
>> submodules and maybe some extra information (is it populated or
>> not, does it have its .git directory embedded, does it contain
>> changes). So maybe "show" should be a slightly pimped "status"?
> 
> Sure.  Do we want to create a new subcommand though?  Aren't "status"
> and "summary" enough already?

Yes, I don't think we need a new command. Maybe someday we will
change "git submodule status" to contain less hash and maybe some
other infos, but we're not there yet.

Re: [PATCH] submodule: make 'show' an alias for 'summary'

[Junio C Hamano]

Jens Lehmann <Jens.Lehmann@web.de> writes:

> Me too would expect a show command to show me a list of all the
> submodules and maybe some extra information (is it populated or
> not, does it have its .git directory embedded, does it contain
> changes). So maybe "show" should be a slightly pimped "status"?

What's the next inventive way somebody who did not read the
documentation will come up with to spell the subcommand known as
"status"?  "submodule list"?

Do we really want 47 different subcommands that give the same output
or slight variations thereof?