* [PATCH v2 0/3] git-submodule: New subcommand 'summary' @ 2008-02-29 17:34 Ping Yin 2008-02-29 17:34 ` [PATCH v2 1/3] git-submodule: New subcommand 'summary' (1) - code framework Ping Yin 0 siblings, 1 reply; 20+ messages in thread From: Ping Yin @ 2008-02-29 17:34 UTC (permalink / raw) To: git; +Cc: gitster This patch series introduces summary support for submodule. ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2 1/3] git-submodule: New subcommand 'summary' (1) - code framework 2008-02-29 17:34 [PATCH v2 0/3] git-submodule: New subcommand 'summary' Ping Yin @ 2008-02-29 17:34 ` Ping Yin 2008-02-29 17:34 ` [PATCH v2 2/3] git-submodule: New subcommand 'summary' (2) - hard work Ping Yin 2008-03-01 7:28 ` [PATCH v2 1/3] git-submodule: New subcommand 'summary' (1) - code framework Junio C Hamano 0 siblings, 2 replies; 20+ messages in thread From: Ping Yin @ 2008-02-29 17:34 UTC (permalink / raw) To: git; +Cc: gitster, Ping Yin Following patches teach git-submodule a new subcommand 'summary' to show commit summary of user-cared (i.e. checked out) submodules since a given commit (default HEAD) of super project. For a submodule in question, a series of commits will be shown as the path from the src commit to the dst commit, where the src commit is from the given super project commit, and the dst commit is from the index or working tree (switched by --cached). .Example: a super project with modified/deleted/typechanged submodules sm1 to sm5. -------------------------------------------- $ git submodule summary # Submodules modifiled: sm1 sm2 sm3 sm4 sm5 # # * sm1 354cd45...3f751e5: # <one line message for C # <one line message for B # >one line message for D # >one line message for E # # * sm2 5c8bfb5...000000: # <one line message for F # # * sm3 354cd45...3f751e5: # Warn: sm3 doesn't contain commit 354cd45 # # * sm4 354cd34(submodule)->235efa(blob): # <one line message for G # # * sm5 354cd34(blob)->235efa(submodule): # >one line message for H # -------------------------------------------- sm1 has commit C as src (given commit or HEAD) and commit E as dst (index or working tree) as following picture shows. --A-->B-->C (in src:354cd45) \ -->D-->E (in dst:3f751e5) The 'Submodules modified' section for sm1 shows how to change sm1 from src commit C to dst commit E: firstly backward (<) to commit A from commit C via commit B, and then forward (>) to commit E via commit D. Illustration for output of deleted sm2 is similar. sm3 and sm4 are submodules with typechanging (blob<->submodule). If the src/dst commit for a submodule is not found in the work tree (say the submodule respository in the work tree), a warning will be issued. sm3 falls into this case. This patch just gives the framework. It just finds the submodules to be shown as follows. -------------------------------------------- $ git submodule summary # Submodules modifiled: sm1 sm2 sm3 sm4 sm5 # -------------------------------------------- Signed-off-by: Ping Yin <pkufranky@gmail.com> --- git-submodule.sh | 70 ++++++++++++++++++++++++++++++++++++++++++++++++++--- 1 files changed, 66 insertions(+), 4 deletions(-) diff --git a/git-submodule.sh b/git-submodule.sh index a6aaf40..8009a8d 100755 --- a/git-submodule.sh +++ b/git-submodule.sh @@ -4,7 +4,7 @@ # # Copyright (c) 2007 Lars Hjemli -USAGE='[--quiet] [--cached] [add <repo> [-b branch]|status|init|update] [--] [<path>...]' +USAGE='[--quiet] [--cached] [add <repo> [-b branch]|status|init|update|summary [<commit>]] [--] [<path>...]' OPTIONS_SPEC= . git-sh-setup require_work_tree @@ -320,7 +320,69 @@ set_name_rev () { ) ) test -z "$revname" || revname=" ($revname)" } +# +# Show commit summary for submodules in index or working tree relative to the given commit +# +# If '--cached' is given, show summary between index and given commit, +# or between working tree and given commit +# +# @ = [head counting commits from (default 'HEAD'),] requested paths (default to all) +# +cmd_summary() +{ + # parse $args after "submodule ... summary". + while test $# -ne 0 + do + case "$1" in + --cached) + cached=1 + ;; + --) + shift + break + ;; + -*) + usage + ;; + *) + break + ;; + esac + shift + done + cache_option=${cached:+--cached} + + if rev=$(git rev-parse --verify "$1^0" 2>/dev/null) + then + head=$rev + shift + else + head=HEAD + fi + + cwd=$(pwd) + cd_to_toplevel + + # Get modified modules cared by user + modules=$(git diff $cache_option --raw $head -- "$@" | + grep '^:160000\|:[0-9]\+ 160000' | + while read mod_src mod_dst sha1_src sha1_dst status name + do + # Always show modules deleted or type-changed (blob<->module) + test $status = D -o $status = T && echo "$name" && continue + # Also show added or modified modules which are checked out + GIT_DIR="$name/.git" git-rev-parse --git-dir >/dev/null 2>&1 && + echo "$name" + done + ) + + # TODO: quote module names containing space or tab + test -n "$modules" && + echo "# Submodules modified: "$modules && + echo "#" + cd "$cwd" +} # # List all submodules, prefixed with: # - submodule not initialized @@ -391,7 +453,7 @@ cmd_status() while test $# != 0 && test -z "$command" do case "$1" in - add | init | update | status) + add | init | update | status | summary) command=$1 ;; -q|--quiet) @@ -430,8 +492,8 @@ then usage fi -# "--cached" is accepted only by "status" -if test -n "$cached" && test "$command" != status +# "--cached" is accepted only by "status" and "summary" +if test -n "$cached" && test "$command" != status -a "$command" != summary then usage fi -- 1.5.4.3.347.g5314c ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v2 2/3] git-submodule: New subcommand 'summary' (2) - hard work 2008-02-29 17:34 ` [PATCH v2 1/3] git-submodule: New subcommand 'summary' (1) - code framework Ping Yin @ 2008-02-29 17:34 ` Ping Yin 2008-02-29 17:34 ` [PATCH v2 3/3] git-submodule: New subcommand 'summary' (3) - limit summary size Ping Yin 2008-03-01 7:29 ` [PATCH v2 2/3] git-submodule: New subcommand 'summary' (2) - hard work Junio C Hamano 2008-03-01 7:28 ` [PATCH v2 1/3] git-submodule: New subcommand 'summary' (1) - code framework Junio C Hamano 1 sibling, 2 replies; 20+ messages in thread From: Ping Yin @ 2008-02-29 17:34 UTC (permalink / raw) To: git; +Cc: gitster, Ping Yin This patch does the hard work of submodule summary and finally gives output shown in last patch. Signed-off-by: Ping Yin <pkufranky@gmail.com> --- git-submodule.sh | 70 +++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 files changed, 69 insertions(+), 1 deletions(-) diff --git a/git-submodule.sh b/git-submodule.sh index 8009a8d..8322771 100755 --- a/git-submodule.sh +++ b/git-submodule.sh @@ -380,7 +380,75 @@ cmd_summary() # TODO: quote module names containing space or tab test -n "$modules" && echo "# Submodules modified: "$modules && - echo "#" + echo "#" && + git diff $cache_option --raw $head -- $modules | + grep '^:160000\|:[0-9]\+ 160000' | + cut -c2- | + while read mod_src mod_dst sha1_src sha1_dst status name + do + sha1_src=$(echo $sha1_src | sed -e 's/\.\.\.//') + sha1_dst=$(echo $sha1_dst | sed -e 's/\.\.\.//') + check_src=$(echo $mod_src | grep 160000) + check_dst=$(echo $mod_dst | grep 160000) + errmsg= + unfound_src= + unfound_dst= + + test -z "$check_src" || + GIT_DIR="$name/.git" git-rev-parse --verify $sha1_src^0 >/dev/null 2>&1 || + unfound_src=t + + test -z "$check_dst" || + GIT_DIR="$name/.git" git-rev-parse --verify $sha1_dst^0 >/dev/null 2>&1 || + unfound_dst=t + + case "$unfound_src,$unfound_dst" in + t,) + errmsg=" Warn: $name doesn't contain commit $sha1_src" + ;; + ,t) + errmsg=" Warn: $name doesn't contain commit $sha1_dst" + ;; + t,t) + errmsg=" Warn: $name doesn't contain commits $sha1_src and $sha1_dst" + ;; + *) + left= + right= + test -n "$check_src" && + left=$(GIT_DIR="$name/.git" git log --pretty=format:" <%s" \ + ${check_dst:+$sha1_dst..}$sha1_src 2>/dev/null) + + test -n "$check_dst" && + right=$(GIT_DIR="$name/.git" git log --reverse --pretty=format:" >%s" \ + ${check_src:+$sha1_src..}$sha1_dst 2>/dev/null) + ;; + esac + + sha1_src=$(echo $sha1_src | cut -c1-7) + sha1_dst=$(echo $sha1_dst | cut -c1-7) + if test $status = T + then + if test $mod_dst = 160000 + then + echo "* $name $sha1_src(blob)->$sha1_dst(submodule):" + else + echo "* $name $sha1_src(submodule)->$sha1_dst(blob):" + fi + else + echo "* $name $sha1_src...$sha1_dst:" + fi + if test -n "$errmsg" + then + # Don't give error msg for modification whose dst is not submodule, i.e. deleted or changed to blob + test $mod_dst = 160000 && echo "$errmsg" + else + test -n "$left" && echo "$left" + test -n "$right" && echo "$right" + fi + echo + done | sed -e 's/^/# /' + cd "$cwd" } # -- 1.5.4.3.347.g5314c ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v2 3/3] git-submodule: New subcommand 'summary' (3) - limit summary size 2008-02-29 17:34 ` [PATCH v2 2/3] git-submodule: New subcommand 'summary' (2) - hard work Ping Yin @ 2008-02-29 17:34 ` Ping Yin 2008-03-01 7:29 ` Junio C Hamano 2008-03-01 7:29 ` [PATCH v2 2/3] git-submodule: New subcommand 'summary' (2) - hard work Junio C Hamano 1 sibling, 1 reply; 20+ messages in thread From: Ping Yin @ 2008-02-29 17:34 UTC (permalink / raw) To: git; +Cc: gitster, Ping Yin This patches teaches git-submodule an option '--summary-limit|-n <number>' to limit number of commits for the summary. Number 0 will disable summary and minus number will not limit the summary size. For beauty and clarification, the last commit for each section (backward and forward) will always be shown disregarding the given limit. So actual summary size may be greater than the given limit. In the same super project of these patch series, 'git submodule -n 2 summary sm1' and 'git submodule -n 3 summary sm1' will show the same. --------------------------------------- $ git submodule -n 2 summary sm1 # Submodules modifiled: sm1 # # * sm1 354cd45...3f751e5: # <one line message for C # <one line message for B # >... (1 more) # >one line message for E # --------------------------------------- Signed-off-by: Ping Yin <pkufranky@gmail.com> --- git-submodule.sh | 56 +++++++++++++++++++++++++++++++++++++++++++++++++++-- 1 files changed, 53 insertions(+), 3 deletions(-) diff --git a/git-submodule.sh b/git-submodule.sh index 8322771..64f1f5c 100755 --- a/git-submodule.sh +++ b/git-submodule.sh @@ -4,7 +4,9 @@ # # Copyright (c) 2007 Lars Hjemli -USAGE='[--quiet] [--cached] [add <repo> [-b branch]|status|init|update|summary [<commit>]] [--] [<path>...]' +USAGE="[--quiet] [--cached] \ +[add <repo> [-b branch]|status|init|update|summary [-n|--summary-limit <number>] [<commit>]] \ +[--] [<path>...]" OPTIONS_SPEC= . git-sh-setup require_work_tree @@ -330,6 +332,8 @@ set_name_rev () { # cmd_summary() { + summary_limit=-1 + # parse $args after "submodule ... summary". while test $# -ne 0 do @@ -337,6 +341,14 @@ cmd_summary() --cached) cached=1 ;; + -n|--summary-limit) + if test -z "$2" || echo "$2" | grep --quiet -v '^-\?[0-9]\+$' + then + usage + fi + summary_limit="$2" + shift + ;; --) shift break @@ -351,6 +363,8 @@ cmd_summary() shift done + test $summary_limit = 0 && return + cache_option=${cached:+--cached} if rev=$(git rev-parse --verify "$1^0" 2>/dev/null) @@ -443,8 +457,44 @@ cmd_summary() # Don't give error msg for modification whose dst is not submodule, i.e. deleted or changed to blob test $mod_dst = 160000 && echo "$errmsg" else - test -n "$left" && echo "$left" - test -n "$right" && echo "$right" + lc0=0 + rc0=0 + test -n "$left" && lc0=$(echo "$left" | wc -l) + test -n "$right" && rc0=$(echo "$right" | wc -l) + + if (( $summary_limit < 0 )) + then + lc=$lc0 + rc=$rc0 + elif (( $lc0 < $summary_limit )) + then + lc=$lc0 + rc=$(($summary_limit-$lc)) + else + lc=$summary_limit + rc=1 + fi + + if (( $rc > $rc0 )) + then + rc=$rc0 + fi + + if test -n "$left" + then + skip=$(($lc0-$lc)) + echo "$left" | head -$(($lc-1)) + test $skip != 0 && echo " <... ($skip more)" + echo "$left" | tail -1 + fi + + if test -n "$right" + then + skip=$(($rc0-$rc)) + echo "$right" | head -$(($rc-1)) + test $skip != 0 && echo " >... ($skip more)" + echo "$right" | tail -1 + fi fi echo done | sed -e 's/^/# /' -- 1.5.4.3.347.g5314c ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH v2 3/3] git-submodule: New subcommand 'summary' (3) - limit summary size 2008-02-29 17:34 ` [PATCH v2 3/3] git-submodule: New subcommand 'summary' (3) - limit summary size Ping Yin @ 2008-03-01 7:29 ` Junio C Hamano [not found] ` <46dff0320803010216m1bd20674if82d2d2072858290@mail.gmail.com> 2008-03-01 12:42 ` Ping Yin 0 siblings, 2 replies; 20+ messages in thread From: Junio C Hamano @ 2008-03-01 7:29 UTC (permalink / raw) To: Ping Yin; +Cc: git Ping Yin <pkufranky@gmail.com> writes: > This patches teaches git-submodule an option '--summary-limit|-n <number>' > to limit number of commits for the summary. Number 0 will disable summary > and minus number will not limit the summary size. "Negative means unlimited" feels unnecessary. Didn't you make "unlimited" the default anyway? > > For beauty and clarification, the last commit for each section (backward > and forward) will always be shown disregarding the given limit. So actual > summary size may be greater than the given limit. > > In the same super project of these patch series, 'git submodule -n 2 > summary sm1' and 'git submodule -n 3 summary sm1' will show the same. This description is unclear. Does "-n 2" tell "show 2 commits from both side", or "show 2 in total"? > --------------------------------------- > $ git submodule -n 2 summary sm1 > # Submodules modifiled: sm1 > # > # * sm1 354cd45...3f751e5: > # <one line message for C > # <one line message for B > # >... (1 more) > # >one line message for E > # When you have room only for N lines, you might have to say (X more), but you never need to say (1 more). You can fit that omitted one item on that line instead of wasting that line to say (1 more). It is unclear from the above illustration as the (1 more) is pointing at right without having anything newer than that, but I think you meant to do something like this: <top <second <... (N more) <bottom >top >second >... (M more) >bottom I guess fork-point may be interesting enough that showing the bottom one like you did might make sense. I am not convinced but we'll see. > + -n|--summary-limit) > + if test -z "$2" || echo "$2" | grep --quiet -v '^-\?[0-9]\+$' \?\+????? summary_limit=$(expr "$2" : '[0-9][0-9]*$') or even if summary_limit=$(( $2 + 0 )) 2>/dev/null || test "$2" != "$summary_limit" then usage fi perhaps. > + if (( $summary_limit < 0 )) Don't. The first line of this script says "#!/bin/sh", not bash. ^ permalink raw reply [flat|nested] 20+ messages in thread
[parent not found: <46dff0320803010216m1bd20674if82d2d2072858290@mail.gmail.com>]
* Re: [PATCH v2 3/3] git-submodule: New subcommand 'summary' (3) - limit summary size [not found] ` <46dff0320803010216m1bd20674if82d2d2072858290@mail.gmail.com> @ 2008-03-01 10:29 ` Ping Yin 0 siblings, 0 replies; 20+ messages in thread From: Ping Yin @ 2008-03-01 10:29 UTC (permalink / raw) To: Git Mailing List On Sat, Mar 1, 2008 at 3:29 PM, Junio C Hamano <gitster@pobox.com> wrote: > Ping Yin <pkufranky@gmail.com> writes: > > > This patches teaches git-submodule an option '--summary-limit|-n <number>' > > to limit number of commits for the summary. Number 0 will disable summary > > and minus number will not limit the summary size. > > "Negative means unlimited" feels unnecessary. Didn't you make "unlimited" > the default anyway? 'unlimited' is the default, i should clarify this in the message. I think 'Negative means unlimited' is neccessary. Someone may override --summary-limit in the shell alias, but sometime he may want to bring back the unlimited behavior in the command line. > > > > For beauty and clarification, the last commit for each section (backward > > and forward) will always be shown disregarding the given limit. So actual > > summary size may be greater than the given limit. > > > > In the same super project of these patch series, 'git submodule -n 2 > > summary sm1' and 'git submodule -n 3 summary sm1' will show the same. > > This description is unclear. Does "-n 2" tell "show 2 commits from both > side", or "show 2 in total"? > I should make it clear that -n means 'in total'. > > > --------------------------------------- > > $ git submodule -n 2 summary sm1 > > # Submodules modifiled: sm1 > > # > > # * sm1 354cd45...3f751e5: > > # <one line message for C > > # <one line message for B > > # >... (1 more) > > # >one line message for E > > # > > When you have room only for N lines, you might have to say (X more), but > you never need to say (1 more). You can fit that omitted one item on that > line instead of wasting that line to say (1 more). > make sense. > > > + -n|--summary-limit) > > + if test -z "$2" || echo "$2" | grep --quiet -v '^-\?[0-9]\+$' > > \?\+????? > > summary_limit=$(expr "$2" : '[0-9][0-9]*$') > > or even > > if summary_limit=$(( $2 + 0 )) 2>/dev/null || > test "$2" != "$summary_limit" > then > usage > fi > > perhaps. > > > + if (( $summary_limit < 0 )) > > Don't. The first line of this script says "#!/bin/sh", not bash. > ok, i'll fix it. -- Ping Yin ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 3/3] git-submodule: New subcommand 'summary' (3) - limit summary size 2008-03-01 7:29 ` Junio C Hamano [not found] ` <46dff0320803010216m1bd20674if82d2d2072858290@mail.gmail.com> @ 2008-03-01 12:42 ` Ping Yin 2008-03-01 20:26 ` Junio C Hamano 1 sibling, 1 reply; 20+ messages in thread From: Ping Yin @ 2008-03-01 12:42 UTC (permalink / raw) To: Junio C Hamano; +Cc: git On Sat, Mar 1, 2008 at 3:29 PM, Junio C Hamano <gitster@pobox.com> wrote: > Ping Yin <pkufranky@gmail.com> writes: > > > + -n|--summary-limit) > > + if test -z "$2" || echo "$2" | grep --quiet -v '^-\?[0-9]\+$' > > \?\+????? > > summary_limit=$(expr "$2" : '[0-9][0-9]*$') > expr is portable? > or even > > if summary_limit=$(( $2 + 0 )) 2>/dev/null || > test "$2" != "$summary_limit" > then > usage > fi > summary_limit=$(( $2 + 0 )) will always has return status 0 So i use summary_limit=$(($2 + 0)) if test $summary_limit = 0 -a "$2" != 0 then usage fi -- Ping Yin ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 3/3] git-submodule: New subcommand 'summary' (3) - limit summary size 2008-03-01 12:42 ` Ping Yin @ 2008-03-01 20:26 ` Junio C Hamano 0 siblings, 0 replies; 20+ messages in thread From: Junio C Hamano @ 2008-03-01 20:26 UTC (permalink / raw) To: Ping Yin; +Cc: git "Ping Yin" <pkufranky@gmail.com> writes: > On Sat, Mar 1, 2008 at 3:29 PM, Junio C Hamano <gitster@pobox.com> wrote: > > expr is portable? It's not just portable but is a very old fashioned and time-proven way to do things like this. >> if summary_limit=$(( $2 + 0 )) 2>/dev/null || >> test "$2" != "$summary_limit" >> then >> usage >> fi >> > > summary_limit=$(( $2 + 0 )) will always has return status 0 Ah, there's a typo there. The intention was to reject non numbers for two in 43 -32 'deadbeef' ' -27' HEAD '' do if sl=$(( $two + 0 )) 2>/dev/null && test "$two" == "$sl" then echo Ah, "$two", that is a number. else echo You gave me an un-number "'$two' (vs '$sl')". fi done ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 2/3] git-submodule: New subcommand 'summary' (2) - hard work 2008-02-29 17:34 ` [PATCH v2 2/3] git-submodule: New subcommand 'summary' (2) - hard work Ping Yin 2008-02-29 17:34 ` [PATCH v2 3/3] git-submodule: New subcommand 'summary' (3) - limit summary size Ping Yin @ 2008-03-01 7:29 ` Junio C Hamano 2008-03-01 11:04 ` Ping Yin 2008-03-07 16:50 ` Ping Yin 1 sibling, 2 replies; 20+ messages in thread From: Junio C Hamano @ 2008-03-01 7:29 UTC (permalink / raw) To: Ping Yin; +Cc: git Ping Yin <pkufranky@gmail.com> writes: > This patch does the hard work of submodule summary and finally gives > output shown in last patch. > > Signed-off-by: Ping Yin <pkufranky@gmail.com> > --- > git-submodule.sh | 70 +++++++++++++++++++++++++++++++++++++++++++++++++++++- > 1 files changed, 69 insertions(+), 1 deletions(-) > > diff --git a/git-submodule.sh b/git-submodule.sh > index 8009a8d..8322771 100755 > --- a/git-submodule.sh > +++ b/git-submodule.sh > @@ -380,7 +380,75 @@ cmd_summary() > # TODO: quote module names containing space or tab > test -n "$modules" && > echo "# Submodules modified: "$modules && > - echo "#" > + echo "#" && > + git diff $cache_option --raw $head -- $modules | > + grep '^:160000\|:[0-9]\+ 160000' | > + cut -c2- | Same issues as before. It might be worthwhile to write this part all in Perl, I suspect... > + while read mod_src mod_dst sha1_src sha1_dst status name > + do > + sha1_src=$(echo $sha1_src | sed -e 's/\.\.\.//') > + sha1_dst=$(echo $sha1_dst | sed -e 's/\.\.\.//') This is unneeded if you used the right tool, i.e. diff-index. > + check_src=$(echo $mod_src | grep 160000) > + check_dst=$(echo $mod_dst | grep 160000) Huh? Do you mean "test $mod_src = ':160000'" and such? > + errmsg= > + unfound_src= > + unfound_dst= Perhaps "missing" is a better word. > + test -z "$check_src" || > + GIT_DIR="$name/.git" git-rev-parse --verify $sha1_src^0 >/dev/null 2>&1 || > + unfound_src=t You do not want to do ^0; you will not be bind a non-commit in gitlink entry anyway. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 2/3] git-submodule: New subcommand 'summary' (2) - hard work 2008-03-01 7:29 ` [PATCH v2 2/3] git-submodule: New subcommand 'summary' (2) - hard work Junio C Hamano @ 2008-03-01 11:04 ` Ping Yin 2008-03-07 16:50 ` Ping Yin 1 sibling, 0 replies; 20+ messages in thread From: Ping Yin @ 2008-03-01 11:04 UTC (permalink / raw) To: Junio C Hamano; +Cc: git On Sat, Mar 1, 2008 at 3:29 PM, Junio C Hamano <gitster@pobox.com> wrote: > Ping Yin <pkufranky@gmail.com> writes: > > > > + check_src=$(echo $mod_src | grep 160000) > > + check_dst=$(echo $mod_dst | grep 160000) > > Huh? Do you mean "test $mod_src = ':160000'" and such? > Yeah. There are 2 places needing test this. I don't find a better way -- Ping Yin ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 2/3] git-submodule: New subcommand 'summary' (2) - hard work 2008-03-01 7:29 ` [PATCH v2 2/3] git-submodule: New subcommand 'summary' (2) - hard work Junio C Hamano 2008-03-01 11:04 ` Ping Yin @ 2008-03-07 16:50 ` Ping Yin 2008-03-07 16:59 ` Jakub Narebski 2008-03-07 18:46 ` Junio C Hamano 1 sibling, 2 replies; 20+ messages in thread From: Ping Yin @ 2008-03-07 16:50 UTC (permalink / raw) To: Junio C Hamano; +Cc: git On Sat, Mar 1, 2008 at 3:29 PM, Junio C Hamano <gitster@pobox.com> wrote: > Ping Yin <pkufranky@gmail.com> writes: > > > + test -z "$check_src" || > > + GIT_DIR="$name/.git" git-rev-parse --verify $sha1_src^0 >/dev/null 2>&1 || > > + unfound_src=t > > You do not want to do ^0; you will not be bind a non-commit in gitlink > entry anyway. > Actually, i need ^0. "git-rev-parse --verify sha1" will always succeed if sha1 is an valid name with lenght 40 even if the sha1 doesn't belong to the repository. But what i want to verify is that the sha1 is not just valid/unique but also belongs to the submodule repository. -- Ping Yin ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 2/3] git-submodule: New subcommand 'summary' (2) - hard work 2008-03-07 16:50 ` Ping Yin @ 2008-03-07 16:59 ` Jakub Narebski 2008-03-07 18:23 ` Ping Yin 2008-03-07 18:46 ` Junio C Hamano 1 sibling, 1 reply; 20+ messages in thread From: Jakub Narebski @ 2008-03-07 16:59 UTC (permalink / raw) To: Ping Yin; +Cc: Junio C Hamano, git "Ping Yin" <pkufranky@gmail.com> writes: > On Sat, Mar 1, 2008 at 3:29 PM, Junio C Hamano <gitster@pobox.com> wrote: >> Ping Yin <pkufranky@gmail.com> writes: >> >>> + test -z "$check_src" || >>> + GIT_DIR="$name/.git" git-rev-parse --verify $sha1_src^0 >/dev/null 2>&1 || >>> + unfound_src=t >> >> You do not want to do ^0; you will not be bind a non-commit in gitlink >> entry anyway. >> > Actually, I need ^0. "git-rev-parse --verify sha1" will always > succeed if sha1 is an valid name with lenght 40 even if the sha1 > doesn't belong to the repository. > > But what I want to verify is that the sha1 is not just valid/unique > but also belongs to the submodule repository. Don't you want "git cat-file -e <sha1>" then, unless you are checking more than one sha1? -- Jakub Narebski Poland ShadeHawk on #git ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 2/3] git-submodule: New subcommand 'summary' (2) - hard work 2008-03-07 16:59 ` Jakub Narebski @ 2008-03-07 18:23 ` Ping Yin 0 siblings, 0 replies; 20+ messages in thread From: Ping Yin @ 2008-03-07 18:23 UTC (permalink / raw) To: Jakub Narebski; +Cc: Junio C Hamano, git On Sat, Mar 8, 2008 at 12:59 AM, Jakub Narebski <jnareb@gmail.com> wrote: > "Ping Yin" <pkufranky@gmail.com> writes: > > > On Sat, Mar 1, 2008 at 3:29 PM, Junio C Hamano <gitster@pobox.com> wrote: > >> Ping Yin <pkufranky@gmail.com> writes: > >> > >>> + test -z "$check_src" || > >>> + GIT_DIR="$name/.git" git-rev-parse --verify $sha1_src^0 >/dev/null 2>&1 || > >>> + unfound_src=t > >> > >> You do not want to do ^0; you will not be bind a non-commit in gitlink > >> entry anyway. > >> > > Actually, I need ^0. "git-rev-parse --verify sha1" will always > > succeed if sha1 is an valid name with lenght 40 even if the sha1 > > doesn't belong to the repository. > > > > But what I want to verify is that the sha1 is not just valid/unique > > but also belongs to the submodule repository. > > Don't you want "git cat-file -e <sha1>" then, unless you are checking > more than one sha1? > Hmm, yeah, the command you give has the same effect with "git-rev-parse --verify sha1^0" > -- > Jakub Narebski > Poland > ShadeHawk on #git > -- Ping Yin ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 2/3] git-submodule: New subcommand 'summary' (2) - hard work 2008-03-07 16:50 ` Ping Yin 2008-03-07 16:59 ` Jakub Narebski @ 2008-03-07 18:46 ` Junio C Hamano 1 sibling, 0 replies; 20+ messages in thread From: Junio C Hamano @ 2008-03-07 18:46 UTC (permalink / raw) To: Ping Yin; +Cc: git "Ping Yin" <pkufranky@gmail.com> writes: > Actually, i need ^0.... > But what i want > to verify is that the sha1 is not just valid/unique > but also belongs to the submodule repository. Ah, I missed that. Thanks for the clarification. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 1/3] git-submodule: New subcommand 'summary' (1) - code framework 2008-02-29 17:34 ` [PATCH v2 1/3] git-submodule: New subcommand 'summary' (1) - code framework Ping Yin 2008-02-29 17:34 ` [PATCH v2 2/3] git-submodule: New subcommand 'summary' (2) - hard work Ping Yin @ 2008-03-01 7:28 ` Junio C Hamano 2008-03-01 10:27 ` Ping Yin [not found] ` <46dff0320803010201q72a72et951e0a3f090684e4@mail.gmail.com> 1 sibling, 2 replies; 20+ messages in thread From: Junio C Hamano @ 2008-03-01 7:28 UTC (permalink / raw) To: Ping Yin; +Cc: git Ping Yin <pkufranky@gmail.com> writes: > Following patches teach git-submodule a new subcommand 'summary' to show > commit summary of user-cared (i.e. checked out) submodules since a given > commit (default HEAD) of super project. > > For a submodule in question, a series of commits will be shown as the path > from the src commit to the dst commit, where the src commit is from the given > super project commit, and the dst commit is from the index or working tree > (switched by --cached). > > .Example: a super project with modified/deleted/typechanged submodules sm1 to sm5. Overlong lines. What's the dot before "Example"? > # * sm2 5c8bfb5...000000: > # <one line message for F > # > # * sm3 354cd45...3f751e5: > # Warn: sm3 doesn't contain commit 354cd45 Nice touch. This can happen even there is no repository corruption. E.g. you can have a checkout of supermodule at a revision newer than your last "git fetch" in sm3. > # * sm4 354cd34(submodule)->235efa(blob): > # <one line message for G > # > # * sm5 354cd34(blob)->235efa(submodule): > # >one line message for H > # > ... > Illustration for output of deleted sm2 is similar. > > sm3 and sm4 are submodules with typechanging (blob<->submodule). Are they? I think you meant 4 and 5. > This patch just gives the framework. It just finds the submodules to be > shown as follows. > > -------------------------------------------- > $ git submodule summary > # Submodules modifiled: sm1 sm2 sm3 sm4 sm5 > # > -------------------------------------------- Probably it would be a better organization to show only this in the commit log message for [1/3] and describe how the output is enhanced in the log message of the commit as the code builds more . > @@ -320,7 +320,69 @@ set_name_rev () { > ) ) > test -z "$revname" || revname=" ($revname)" > } > +# > +# Show commit summary for submodules in index or working tree relative to the given commit > +# > +# If '--cached' is given, show summary between index and given commit, > +# or between working tree and given commit Overlong lines again. > +# @ = [head counting commits from (default 'HEAD'),] requested paths (default to all) > +# What's "@ =" convention? > +cmd_summary() > +{ We seem to have '{' on the same line for shell functions in our scripts. > + # parse $args after "submodule ... summary". > + while test $# -ne 0 > + do > + case "$1" in > + --cached) > + cached=1 > + ;; If you do this "cached="$1" instead here, then you do not need to do ... > + cache_option=${cached:+--cached} ... this. > + > + if rev=$(git rev-parse --verify "$1^0" 2>/dev/null) > + then > + head=$rev > + shift > + else > + head=HEAD > + fi Hmph, is showing diff with anything other than HEAD useful? What happens if the user says "git submodules status HAED" by mistake? > + > + cwd=$(pwd) > + cd_to_toplevel > + > + # Get modified modules cared by user > + modules=$(git diff $cache_option --raw $head -- "$@" | When scripting, please do not use "git diff" unless absolutely necessary. Its output is not meant for script consumption and can be made more "user friendly" as user request comes in. Instead, use "git diff-index" here. > + grep '^:160000\|:[0-9]\+ 160000' | This looks troublesome. - [0-9] is obviously wrong and [0-7] is what you meant; - \| and \+ are not BRE but GNU. It probably is more portable if written like this: grep -e '^:160000 ' -e '^:[0-7]* 160000 ' > + # TODO: quote module names containing space or tab Yeah, the scripted version is bound to have a lot of issues with funny characters, but let's not worry about it too much. It is something we can fix later and more easily when we rewrite it in C (or Perl), but first get the feature right and stable. > + test -n "$modules" && > + echo "# Submodules modified: "$modules && > + echo "#" > + cd "$cwd" Hmph, is there any point to try coming back there? You could have even done the cd-to-toplevel inside of $( ... ) construct which is run inside a subshell, so... ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 1/3] git-submodule: New subcommand 'summary' (1) - code framework 2008-03-01 7:28 ` [PATCH v2 1/3] git-submodule: New subcommand 'summary' (1) - code framework Junio C Hamano @ 2008-03-01 10:27 ` Ping Yin 2008-03-01 20:52 ` Junio C Hamano [not found] ` <46dff0320803010201q72a72et951e0a3f090684e4@mail.gmail.com> 1 sibling, 1 reply; 20+ messages in thread From: Ping Yin @ 2008-03-01 10:27 UTC (permalink / raw) To: Junio C Hamano; +Cc: Git Mailing List > > > > + grep '^:160000\|:[0-9]\+ 160000' | > > This looks troublesome. > > - [0-9] is obviously wrong and [0-7] is what you meant; > - \| and \+ are not BRE but GNU. > man grep says In basic regular expressions the metacharacters ?, +, {, |, (, and ) lose their special meaning; instead use the backslashed versions \?, \+, \{, \|, \(, and \). Doen't it mean that '\|' is BRE ? -- Ping Yin ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 1/3] git-submodule: New subcommand 'summary' (1) - code framework 2008-03-01 10:27 ` Ping Yin @ 2008-03-01 20:52 ` Junio C Hamano 2008-03-02 2:16 ` Junio C Hamano 2008-03-02 5:11 ` Ping Yin 0 siblings, 2 replies; 20+ messages in thread From: Junio C Hamano @ 2008-03-01 20:52 UTC (permalink / raw) To: Ping Yin; +Cc: Git Mailing List "Ping Yin" <pkufranky@gmail.com> writes: >> > + grep '^:160000\|:[0-9]\+ 160000' | >> >> This looks troublesome. >> >> - [0-9] is obviously wrong and [0-7] is what you meant; >> - \| and \+ are not BRE but GNU. >> > man grep says > In basic regular expressions the metacharacters ?, +, {, |, (, and ) > lose their special meaning; instead use the backslashed versions \?, > \+, \{, \|, \(, and \). > > Doen't it mean that '\|' is BRE ? It just says unlike in ERE, these characters are not special in BRE; it does not at all say using backslash like \?, \+, and \| makes them so. And they are not. \(...\), \{m\}, \{m,\} and \{m,n\} are part of BRE, but the two you used (\+ and \|) are not. GNU accept these two as extensions, but other POSIX implementations may have troubles with them. http://www.opengroup.org/onlinepubs/000095399/basedefs/xbd_chap09.html Please be gentle to porters to non GNU systems. Either stay inside BRE (which I think we have managed to do with our usage of grep) or explicitly ask for ERE with "grep -E". ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 1/3] git-submodule: New subcommand 'summary' (1) - code framework 2008-03-01 20:52 ` Junio C Hamano @ 2008-03-02 2:16 ` Junio C Hamano 2008-03-02 5:11 ` Ping Yin 1 sibling, 0 replies; 20+ messages in thread From: Junio C Hamano @ 2008-03-02 2:16 UTC (permalink / raw) To: Ping Yin; +Cc: Git Mailing List Junio C Hamano <gitster@pobox.com> writes: > "Ping Yin" <pkufranky@gmail.com> writes: > ... >> man grep says >> In basic regular expressions the metacharacters ?, +, {, |, (, and ) >> lose their special meaning; instead use the backslashed versions \?, >> \+, \{, \|, \(, and \). >> >> Doen't it mean that '\|' is BRE ? > > It just says unlike in ERE, these characters are not special in BRE; it > does not at all say using backslash like \?, \+, and \| makes them so. > > And they are not. \(...\), \{m\}, \{m,\} and \{m,n\} are part of BRE, but > the two you used (\+ and \|) are not. GNU accept these two as extensions, > but other POSIX implementations may have troubles with them. > > http://www.opengroup.org/onlinepubs/000095399/basedefs/xbd_chap09.html > > Please be gentle to porters to non GNU systems. Either stay inside BRE > (which I think we have managed to do with our usage of grep) or explicitly > ask for ERE with "grep -E". I think this would help people new to our codebase. Also Message-ID: <7v4pdtgjf0.fsf@gitster.siamese.dyndns.org> Subject: Re: [PATCH] git-filter-branch could be confused by similar names Date: Fri, 04 Jan 2008 17:17:39 -0800 aka http://article.gmane.org gmane.comp.version-control.git/69630 --- Documentation/CodingGuidelines | 12 ++++++++++++ 1 files changed, 12 insertions(+), 0 deletions(-) diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines index 3b042db..994eb91 100644 --- a/Documentation/CodingGuidelines +++ b/Documentation/CodingGuidelines @@ -53,6 +53,18 @@ For shell scripts specifically (not exhaustive): - We do not write the noiseword "function" in front of shell functions. + - As to use of grep, stick to a subset of BRE (namely, no \{m,n\}, + [::], [==], nor [..]) for portability. + + - We do not use \{m,n\}; + + - We do not use -E; + + - We do not use ? nor + (which are \{0,1\} and \{1,\} + respectively in BRE) but that goes without saying as these + are ERE elements not BRE (note that \? and \+ are not even part + of BRE -- making them accessible from BRE is a GNU extension). + For C programs: - We use tabs to indent, and interpret tabs as taking up to ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH v2 1/3] git-submodule: New subcommand 'summary' (1) - code framework 2008-03-01 20:52 ` Junio C Hamano 2008-03-02 2:16 ` Junio C Hamano @ 2008-03-02 5:11 ` Ping Yin 1 sibling, 0 replies; 20+ messages in thread From: Ping Yin @ 2008-03-02 5:11 UTC (permalink / raw) To: Junio C Hamano; +Cc: Git Mailing List On Sun, Mar 2, 2008 at 4:52 AM, Junio C Hamano <gitster@pobox.com> wrote: > "Ping Yin" <pkufranky@gmail.com> writes: > > > > >> > + grep '^:160000\|:[0-9]\+ 160000' | > >> > >> This looks troublesome. > >> > >> - [0-9] is obviously wrong and [0-7] is what you meant; > >> - \| and \+ are not BRE but GNU. > >> > > man grep says > > In basic regular expressions the metacharacters ?, +, {, |, (, and ) > > lose their special meaning; instead use the backslashed versions \?, > > \+, \{, \|, \(, and \). > > > > Doen't it mean that '\|' is BRE ? > > It just says unlike in ERE, these characters are not special in BRE; it > does not at all say using backslash like \?, \+, and \| makes them so. > > And they are not. \(...\), \{m\}, \{m,\} and \{m,n\} are part of BRE, but > the two you used (\+ and \|) are not. GNU accept these two as extensions, > but other POSIX implementations may have troubles with them. > > http://www.opengroup.org/onlinepubs/000095399/basedefs/xbd_chap09.html > > Please be gentle to porters to non GNU systems. Either stay inside BRE > (which I think we have managed to do with our usage of grep) or explicitly > ask for ERE with "grep -E". > ok, 3x. -- Ping Yin ^ permalink raw reply [flat|nested] 20+ messages in thread
[parent not found: <46dff0320803010201q72a72et951e0a3f090684e4@mail.gmail.com>]
* Re: [PATCH v2 1/3] git-submodule: New subcommand 'summary' (1) - code framework [not found] ` <46dff0320803010201q72a72et951e0a3f090684e4@mail.gmail.com> @ 2008-03-01 10:28 ` Ping Yin 0 siblings, 0 replies; 20+ messages in thread From: Ping Yin @ 2008-03-01 10:28 UTC (permalink / raw) To: Git Mailing List On Sat, Mar 1, 2008 at 3:28 PM, Junio C Hamano <gitster@pobox.com> wrote: > > Ping Yin <pkufranky@gmail.com> writes: > > .Example: a super project with modified/deleted/typechanged submodules sm1 to sm5. > > Overlong lines. What's the dot before "Example"? '.' is an asciitext syntax. Maybe i should not use it in a commit message. > > > > sm3 and sm4 are submodules with typechanging (blob<->submodule). > > Are they? I think you meant 4 and 5. > oh, it's a typo > > > This patch just gives the framework. It just finds the submodules to be > > shown as follows. > > > > -------------------------------------------- > > $ git submodule summary > > # Submodules modifiled: sm1 sm2 sm3 sm4 sm5 > > # > > -------------------------------------------- > > Probably it would be a better organization to show only this in the commit > log message for [1/3] and describe how the output is enhanced in the log > message of the commit as the code builds more . > Nice suggestion. Should i move most text into man page instead of in commit message? > > > +# @ = [head counting commits from (default 'HEAD'),] requested paths (default to all) > > +# > > What's "@ =" convention? typo again, should be $@ > > > +cmd_summary() > > +{ > > We seem to have '{' on the same line for shell functions in our scripts. > ok > > > + # parse $args after "submodule ... summary". > > + while test $# -ne 0 > > + do > > + case "$1" in > > + --cached) > > + cached=1 > > + ;; > > If you do this "cached="$1" instead here, then you do not need to do ... > > > + cache_option=${cached:+--cached} > > ... this. > --cached may be passed before 'summary' subcommand. So in the outer option parsing should i replace cached=1 to cached=$1 just in this patch or in another patch? > > > + > > + if rev=$(git rev-parse --verify "$1^0" 2>/dev/null) > > + then > > + head=$rev > > + shift > > + else > > + head=HEAD > > + fi > > Hmph, is showing diff with anything other than HEAD useful? What happens > if the user says "git submodules status HAED" by mistake? > s/status/summary ? This patch has nothing to do with git submodule status. 'git submodule summary' and 'git submodule summary HEAD' is equivalent Except HEAD, HEAD^ is neccessary. Since my target is to call 'git submodule summary' in wt-status.c and teach git-status and git-commit show this summary. So when 'git commit --amend', 'git submodule summary HEAD^' should be used. > > > + > > + cwd=$(pwd) > > + cd_to_toplevel > > + > > + # Get modified modules cared by user > > + modules=$(git diff $cache_option --raw $head -- "$@" | > > When scripting, please do not use "git diff" unless absolutely necessary. > Its output is not meant for script consumption and can be made more "user > friendly" as user request comes in. Instead, use "git diff-index" here. ok > > > + test -n "$modules" && > > + echo "# Submodules modified: "$modules && > > + echo "#" > > + cd "$cwd" > > Hmph, is there any point to try coming back there? You could have even > done the cd-to-toplevel inside of $( ... ) construct which is run inside a > subshell, so... > Hmm, just to avoid side effect of this function. I'll put 'cd' into the subshell -- Ping Yin ^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2008-03-07 18:47 UTC | newest] Thread overview: 20+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2008-02-29 17:34 [PATCH v2 0/3] git-submodule: New subcommand 'summary' Ping Yin 2008-02-29 17:34 ` [PATCH v2 1/3] git-submodule: New subcommand 'summary' (1) - code framework Ping Yin 2008-02-29 17:34 ` [PATCH v2 2/3] git-submodule: New subcommand 'summary' (2) - hard work Ping Yin 2008-02-29 17:34 ` [PATCH v2 3/3] git-submodule: New subcommand 'summary' (3) - limit summary size Ping Yin 2008-03-01 7:29 ` Junio C Hamano [not found] ` <46dff0320803010216m1bd20674if82d2d2072858290@mail.gmail.com> 2008-03-01 10:29 ` Ping Yin 2008-03-01 12:42 ` Ping Yin 2008-03-01 20:26 ` Junio C Hamano 2008-03-01 7:29 ` [PATCH v2 2/3] git-submodule: New subcommand 'summary' (2) - hard work Junio C Hamano 2008-03-01 11:04 ` Ping Yin 2008-03-07 16:50 ` Ping Yin 2008-03-07 16:59 ` Jakub Narebski 2008-03-07 18:23 ` Ping Yin 2008-03-07 18:46 ` Junio C Hamano 2008-03-01 7:28 ` [PATCH v2 1/3] git-submodule: New subcommand 'summary' (1) - code framework Junio C Hamano 2008-03-01 10:27 ` Ping Yin 2008-03-01 20:52 ` Junio C Hamano 2008-03-02 2:16 ` Junio C Hamano 2008-03-02 5:11 ` Ping Yin [not found] ` <46dff0320803010201q72a72et951e0a3f090684e4@mail.gmail.com> 2008-03-01 10:28 ` Ping Yin
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).