Re: [PATCH v2 3/3] git-submodule: New subcommand 'summary' (3) - limit summary size

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 1, 2008, 07:29 UTC
Message-ID: <7v1w6u7vhf.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <18af168b52a735c33612c9c9e4778d8b8bef1cbc.1204306070.git.pkufranky@gmail.com>

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?

Show 7 quoted lines

>
> 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"?

Show 10 quoted lines

> ---------------------------------------
>  $ 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.