threads / patch / 31674

patchsubmodule: use abbreviated sha1 in 'status' output

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

## tl;dr

15 messages between Sep 29, 2012 and Oct 1, 2012. Diffs are folded; open one to read it.

replies: 14people: 3as markdown or json

Ramkumar Ramachandra· Sep 29, 2012, 13:43 UTC · lore

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

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(-)
Show changes to 2 files +10 −2

Documentation/git-submodule.txt, git-submodule.sh

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
Ramkumar Ramachandra· Sep 29, 2012, 13:43 UTC · re: Ramkumar Ramachandra · lore
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 git-submodule.sh |   15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)
Show changes to git-submodule.sh +8 −7
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
Jens Lehmann· Sep 29, 2012, 14:31 UTC · re: Ramkumar Ramachandra · lore

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

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:
Show 46 quoted lines
> 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"
>
Ramkumar Ramachandra· Sep 29, 2012, 14:45 UTC · re: Jens Lehmann · lore

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

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
Jens Lehmann· Sep 29, 2012, 15:05 UTC · re: Ramkumar Ramachandra · lore

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

Am 29.09.2012 16:45, schrieb Ramkumar Ramachandra:
Show 8 quoted lines
> 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.

Ramkumar Ramachandra· Oct 1, 2012, 06:42 UTC · re: Jens Lehmann · lore

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

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
Jens Lehmann· Oct 1, 2012, 17:22 UTC · re: Ramkumar Ramachandra · lore

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

Am 01.10.2012 08:42, schrieb Ramkumar Ramachandra:
Show 7 quoted lines
> 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.

Jens Lehmann· Sep 29, 2012, 14:49 UTC · re: Ramkumar Ramachandra · lore

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

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?

Ramkumar Ramachandra· Sep 29, 2012, 15:07 UTC · re: Jens Lehmann · lore

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

Hi Jens,
Jens Lehmann wrote:
Show 12 quoted lines
> 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
Jens Lehmann· Sep 29, 2012, 15:27 UTC · re: Ramkumar Ramachandra · lore

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

Am 29.09.2012 17:07, schrieb Ramkumar Ramachandra:
Show 19 quoted lines
> 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)?

Ramkumar Ramachandra· Oct 1, 2012, 06:45 UTC · re: Jens Lehmann · lore

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

Hi Jens,
Jens Lehmann wrote:
Show 7 quoted lines
> 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
Jens Lehmann· Oct 1, 2012, 17:26 UTC · re: Ramkumar Ramachandra · lore

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

Am 01.10.2012 08:45, schrieb Ramkumar Ramachandra:
Show 12 quoted lines
> 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"?

Ramkumar Ramachandra· Oct 1, 2012, 17:33 UTC · re: Jens Lehmann · lore

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

Hi Jens,
Jens Lehmann wrote:
Show 17 quoted lines
> 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
Jens Lehmann· Oct 1, 2012, 19:31 UTC · re: Ramkumar Ramachandra · lore

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

Am 01.10.2012 19:33, schrieb Ramkumar Ramachandra:
Show 20 quoted lines
> 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? ;-)

Show 10 quoted lines
>> 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.

Show 7 quoted lines
>> 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.

Junio C Hamano· Oct 1, 2012, 17:41 UTC · re: Jens Lehmann · lore

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

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?

← back to recent threads