From: "Ping Yin" <pkufranky@gmail.com>
To: "Git Mailing List" <git@vger.kernel.org>
Subject: Re: [PATCH v2 1/3] git-submodule: New subcommand 'summary' (1) - code framework
Date: Sat, 1 Mar 2008 18:28:14 +0800 [thread overview]
Message-ID: <46dff0320803010228g2d4672c3qd0548133d43b11a@mail.gmail.com> (raw)
In-Reply-To: <46dff0320803010201q72a72et951e0a3f090684e4@mail.gmail.com>
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
prev parent reply other threads:[~2008-03-01 10:28 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
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 message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=46dff0320803010228g2d4672c3qd0548133d43b11a@mail.gmail.com \
--to=pkufranky@gmail.com \
--cc=git@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
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 read-only IMAP folder(s) and NNTP newsgroup(s).