From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: Harry Jeffery <harry@exec64.co.uk>, git@vger.kernel.org
Subject: Re: [PATCH] pretty-format: add append line-feed format specifier
Date: Tue, 9 Sep 2014 17:45:21 -0400 [thread overview]
Message-ID: <20140909214520.GA13603@peff.net> (raw)
In-Reply-To: <xmqqegvkk2k3.fsf@gitster.dls.corp.google.com>
On Tue, Sep 09, 2014 at 12:37:48PM -0700, Junio C Hamano wrote:
> Harry Jeffery <harry@exec64.co.uk> writes:
>
> > On 09/09/14 20:15, Junio C Hamano wrote:
> >> Is this different from "%n%-d"?
> >>
> >
> > Yes. "%n%-d" will place the newline before the expansion, not after.
>
> Maybe "%[-+ ]" needs to be rethought, instead of making things worse
> by turning it into "%[-_+ ]", as the next person who comes would
> want to add space after the expansion and would need to find yet
> another letter like you did with '_'.
Yeah, that was my thought on reading the initial patch, too. Why limit
ourselves to newlines and spaces. I'd much rather have full conditional
expansion, like "${foo:+prefix $foo suffix}" in the shell.
Something like the patch below might work, but I didn't test it very
thoroughly (and note the comments, which might need dealing with). Maybe
it would make a sensible base for Harry to build on if he wants to
pursue this.
With it, you can do:
git log --format='%h %s%if(%d,%n Decoration:%d)' origin
to get:
85f0837 Start the post-2.1 cycle
Decoration: (origin/master, origin/HEAD, github/foo)
f655651 Merge branch 'rs/strbuf-getcwd'
51eeaea Merge branch 'ta/pretty-parse-config'
4740891 Merge branch 'bc/archive-pax-header-mode'
0e28161 Merge branch 'pr/remotes-in-hashmap'
44ceb79 Merge branch 'jk/pretty-empty-format'
56f214e Merge branch 'ta/config-set'
e8e4ce7 Merge branch 'rs/init-no-duplicate-real-path'
1d8a6f6 Merge branch 'mm/config-edit-global'
c518279 Merge branch 'jc/reopen-lock-file'
96db324 Merge git://github.com/git-l10n/git-po
Decoration: (origin/maint)
You could also make "%d" more flexible with it. We unconditionally
include the " (...)" wrapper when expanding it. But assuming we
introduced a "%D" that is _just_ the decoration names, you could do:
%if(%D, (%D))
to get the same effect with much more flexibility.
---
diff --git a/pretty.c b/pretty.c
index fe34ddc..96cd512 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1398,6 +1398,42 @@ static size_t format_commit_item(struct strbuf *sb, /* in UTF-8 */
ADD_SP_BEFORE_NON_EMPTY
} magic = NO_MAGIC;
+ if (starts_with(placeholder, "if(")) {
+ const char *cond_beg, *cond_end;
+ const char *data_beg, *data_end;
+ char *buf;
+ struct strbuf scratch = STRBUF_INIT;
+
+ /* can't handle commas in conditions; allow backslash-escaping? */
+ cond_beg = cond_end = placeholder + 3;
+ for (cond_end = cond_beg; *cond_end != ','; cond_end++)
+ if (!*cond_end)
+ return 0;
+
+ /* ditto for nested parentheses; backslash escaping? */
+ data_beg = cond_end + 1;
+ for (data_end = data_beg; *data_end != ')'; data_end++)
+ if (!*data_end)
+ return 0;
+
+ /*
+ * Should teach formatters to return size only without
+ * actually writing to scratch buffer?
+ */
+ buf = xmemdupz(cond_beg, cond_end - cond_beg);
+ strbuf_expand(&scratch, buf, format_commit_item, context);
+ free(buf);
+
+ if (scratch.len) {
+ buf = xmemdupz(data_beg, data_end - data_beg);
+ strbuf_expand(sb, buf, format_commit_item, context);
+ free(buf);
+ }
+ strbuf_release(&scratch);
+
+ return data_end - placeholder + 1;
+ }
+
switch (placeholder[0]) {
case '-':
magic = DEL_LF_BEFORE_EMPTY;
next prev parent reply other threads:[~2014-09-09 21:45 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-09-09 18:09 [PATCH] pretty-format: add append line-feed format specifier Harry Jeffery
2014-09-09 19:15 ` Junio C Hamano
2014-09-09 19:30 ` Harry Jeffery
2014-09-09 19:37 ` Junio C Hamano
2014-09-09 21:45 ` Jeff King [this message]
2014-09-09 22:17 ` Harry Jeffery
2014-09-09 22:31 ` Jeff King
2014-09-10 17:19 ` Junio C Hamano
2014-09-12 4:49 ` Jeff King
2014-09-12 16:36 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20140909214520.GA13603@peff.net \
--to=peff@peff.net \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=harry@exec64.co.uk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).