From: Junio C Hamano <gitster@pobox.com>
To: Ping Yin <pkufranky@gmail.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH v2 1/3] git-submodule: New subcommand 'summary' (1) - code framework
Date: Fri, 29 Feb 2008 23:28:44 -0800 [thread overview]
Message-ID: <7vk5km7vir.fsf@gitster.siamese.dyndns.org> (raw)
In-Reply-To: 80aa1c46ced6f0b92ca2fca3b917d383343b3161.1204306070.git.pkufranky@gmail.com
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...
next prev parent reply other threads:[~2008-03-01 7:29 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 ` Junio C Hamano [this message]
2008-03-01 10:27 ` [PATCH v2 1/3] git-submodule: New subcommand 'summary' (1) - code framework 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
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=7vk5km7vir.fsf@gitster.siamese.dyndns.org \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=pkufranky@gmail.com \
/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).