On Sat, Mar 1, 2008 at 3:28 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
>
> Ping Yin <pkufranky@xxxxxxxxx> 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
