Re: Fwd: [PATCH 2/2] pretty.c: allow date formats in user format strings

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

 



On Mon, Mar 07, 2011 at 06:50:34PM +0000, Will Palmer wrote:

> I'm home now, and apparently that should have been:
> https://github.com/wpalmer/git/tree/pretty/parse-format
> 
> I assume the code is very hard to follow, as it was pretty much written
> with the mindset of "get it done now, fix it later". Looking into it
> again, I see that part of the reason I abandoned it was not being able
> to determine a good way to split things into logical commits. It's
> almost entirely an "everything works or nothing works" change.

I haven't looked at your code yet, but the breakdown of patches I would
expect / hope for is something like:

  1. introduce infrastructure for creating parse-tree from strbuf_expand
     format, with some tests

  2. port format_commit_* over to new system; I would expect that the
     caller code will have to be part of both the parsing and the
     expansion, since the generic code can't know that "%ad" is
     meaningful (and we want to keep it for backwards compatibility).
     Leave format_commit_message as a parse + expand wrapper for simple
     callers who don't care about speed.

  3. Add generic "%(key:option)" support to the new infrastructure,
     forward-porting format_commit_* as necessary (and hopefully the
     change are minimal...).

So those are all big commits, obviously, but hopefully it lets us review
in three stages: does the new infrastructure look good, does porting an
existing caller (and probably the most complex caller) clean up the
caller code, and then finally, does the new syntax look good?

But of course the devil is in the details, so probably that breakdown
has some flaw in it. :) I'll see when I look at your code how close to
reality I came.

-Peff
--
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]