On Wed, 2014-07-30 at 20:09 -0400, Jeff King wrote:
> On Wed, Jul 30, 2014 at 12:43:09PM -0700, Junio C Hamano wrote:
>
> > > "git log --cc" is one of the things I wanted for a long time to fix.
> > > When the user explicitly asks "--cc", we currently ignore it, but
> > > because we know the user wants to view combined diff, we should turn
> > > "-p" on automatically. And the change this patch introduces will be
> > > broken when we fix "log --cc" ("stash list" will end up always
> > > showing the patch, without a way to disable it).
> > >
> > > Can you make this conditional? Do this only when <options> are
> > > given to "git stash list" command and that includes "-p" or
> > > something?
>
> I'm definitely sympathetic. Using "--cc" with the diff family _does_
> imply "-p" already, so it would be nice to do the same for the log
> family.
>
> > Perhaps something along this line?
> >
> > git-stash.sh | 11 +++++++++--
> > 1 file changed, 9 insertions(+), 2 deletions(-)
> >
> > diff --git a/git-stash.sh b/git-stash.sh
> > index ae73ba4..0db1b19 100755
> > --- a/git-stash.sh
> > +++ b/git-stash.sh
> > @@ -297,8 +297,15 @@ have_stash () {
> >
> > list_stash () {
> > have_stash || return 0
> > - git log --format="%gd: %gs" -g --cc --simplify-combined-diff \
> > - "$@" $ref_stash --
> > + case " $* " in
> > + *' --cc '*)
> > + ;; # the user knows what she is doing
> > + *' -p '* | *' -U'*)
> > + set x "--cc" "--simplify-combined-diff" "$@"
> > + shift
> > + ;;
> > + esac
> > + git log --format="%gd: %gs" -g "$@" $ref_stash --
>
> Ugh. I'd generally like to avoid this sort of ad-hoc command line
> parsing, as it is easy to get confused by option arguments or
> even non-options. At least we do not have to worry about pathspecs here,
> as we already do an explicit "--" (so we might be confused by them, but
> they are broken anyway).
>
> Still, I wonder if a cleaner solution is to provide alternate "default
> to this" options for the revision.c option parser. We already have
> "--default" to pick the default starting point. Could we do something
> like "--default-merge-handling=cc" or something?
>
> There's a similar ad-hoc parsing case in "stash show", where we add
> "--stat" if no arguments are given, but we have no clue if a diff format
> was given (so "git stash show --color" accidentally turns on patch
> format!). Something like "--default-diff-format=stat" would be more
> robust.
>
> I dunno. Maybe it is over-engineering. I just hate to see solutions that
> work most of the time and crumble in weird corner cases, even if people
> don't hit those corner cases very often.
>
> -Peff
I agree with you on this one. Those corner cases tend to bite the worst.
Thanks,
Jake
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
