Re: [PATCH v2 0/5] Pretty formats for reflog data

On Fri, Oct 16, 2009 at 12:41:43AM +0200, Thomas Rast wrote:

Jeff King wrote:

quoted

Maybe a better solution would be a "short name" variant for pretty
format specifiers. We already have %(refname) and %(refname:short) [...]
The tricky part would be deciding on a syntax. This seems to come up a
fair bit.

Ok, I settled for %g[dDs] for now to save on letters.  I'm saving the
syntax question for a later series while we discuss this one ;-)

:) That is probably sensible. Your %g[dD] doesn't support selecting
between the numbered version and the "date" version, which is something
we might want, but certainly it is no worse than the status quo (and
doing something like that probably _would_ want an extended syntax, as
you now have two orthogonal arguments: shorten and date/numbered).

I think going for %(...) wouldn't be too bad since we already have
that in for-each-ref, and it can be backwards compatible.  So we would
have different sets of short and long specifiers, e.g.

  %ae = %(authoremail)
  %aE = %(authoremail:mailmap)

We can then pass arguments via some yet-to-be decided syntax, say,
%(body:indent(10)).

That seems reasonable to me, though if we can limit ourselves to one
argument per specifier (I suspect most specifiers would simply be
boolean, but a few may take numbers or strings), then something like
%(body:indent=10) might be a little more readable.

It would also be nice to have some sort of conditional inclusion, which
could deal with your extra ": " in patch 3. Either something like:

  %(reflog:short)%(reflog:+: )

or even

  %(reflog:short:prefix=\: )

and note that allowing arbitrary arguments means we get to deal with
quoting.

But that is all for another potential series.

Other changes in this version include:

* I followed your struct suggestion, which is the new 1/5.

Thanks. It looks like it didn't turn out to be too invasive, and I think
some of the callsites are a bit more readable.

* I fixed the warning that Junio found (and finally found the right
  combination of -W flags, though I cannot compile with -Werror myself
  because of *other* warnings...)

I always compile with -Wall -Werror (including testing your series);
what warnings are you getting?

Thomas Rast (5):
  Refactor pretty_print_commit arguments into a struct
  reflog-walk: refactor the branch@{num} formatting
  Introduce new pretty formats %g[sdD] for reflog information
  stash list: use new %g formats instead of sed
  stash list: drop the default limit of 10 stashes

Thanks, this series looks really good to me. I have a few comments on
patch 3 which I'll send separately.

-Peff