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

On Sat, Mar 1, 2008 at 3:29 PM, Junio C Hamano [off-list ref] wrote:
 > Ping Yin [off-list ref] 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?
 'unlimited' is the default, i should clarify this in the message.

 I think 'Negative means unlimited' is neccessary. Someone may override
 --summary-limit in the shell alias, but sometime he may want to  bring
 back the unlimited behavior in the command line.


 > >
 >  > 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"?
 >
 I should make it clear that -n means '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).
 >
 make sense.


 >
 >  > +             -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.
 >
 ok, i'll fix it.



 --
 Ping Yin