git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
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;

  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).