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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Ping Yin <pkufranky@xxxxxxxxx> 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.
--
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

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux