Re: [PATCH] pretty-format: add append line-feed format specifier
Jeff King p...@peff.net writes: I dunno. I wrote that original set of lua pretty-format patches to try to stop the insanity once and for all. But I realized that I do not actually want to do anything complicated with the output formats, and --oneline and a few simple --format calls usually are enough. Yeah, I share that exactly, and %+ and friends are meant to be on this side of the line between a few simple and complicated. I was not sure which side %if() falls myself, and while I still am not sure, if I were asked to decide which today, I would probably say it is on the simple side. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] pretty-format: add append line-feed format specifier
On Wed, Sep 10, 2014 at 10:19:21AM -0700, Junio C Hamano wrote: Jeff King p...@peff.net writes: 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 ... 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. Yup. I do not think we need to go overboard to support nesting and stuff, let alone turing completeness ;-), especially when we are going to test the condition part only for emptyness. Even with this simple patch, I sense that we are near a slipperly slope of wanting to add %unless(%d, ) or %ifelse(%d,%d, \(undefined\)), so I am not 100% convinced yet. What compelled me is the fact that we already started down the slippery slope with %+ and friends. Providing conditionals but then only allowing certain characters seems weirdly limiting. But I guess it is all part of the same slope. I dunno. I wrote that original set of lua pretty-format patches to try to stop the insanity once and for all. But I realized that I do not actually want to do anything complicated with the output formats, and --oneline and a few simple --format calls usually are enough. And if I do want more, I pipe it into a perl script (typically using --format to make it simple to parse). We could also provide the data in some standard structured format like JSON to make the pipe to your language of choice option easier on people. But using custom --formats to do so is not that hard, and is way more efficient than dumping all of the data. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] pretty-format: add append line-feed format specifier
Jeff King p...@peff.net writes: 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 ... 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. Yup. I do not think we need to go overboard to support nesting and stuff, let alone turing completeness ;-), especially when we are going to test the condition part only for emptyness. Even with this simple patch, I sense that we are near a slipperly slope of wanting to add %unless(%d, ) or %ifelse(%d,%d, \(undefined\)), so I am not 100% convinced yet. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] pretty-format: add append line-feed format specifier
Harry Jeffery ha...@exec64.co.uk writes: Add a new format prefix `_` that causes a line-feed to be inserted immediately after an expansion if the expansion expands to a non-empty string. This is useful for when you would like a line for an expansion to be prepended, but only when the expansion expands to a non empty string, such as inserting a '%_d' expansion before a commit to show any refs pointing towards it. Is this different from %n%-d? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] pretty-format: add append line-feed format specifier
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. log --decorate --pretty=format:%n%-d%h\\ %t\\ [%cn]\\ %s --- (HEAD, upstream/master, master)85f0837 c29da1d [Junio C Hamano] Start the post-2.1 cycle f655651 4027a43 [Junio C Hamano] Merge branch 'rs/strbuf-getcwd' 51eeaea 1f4970c [Junio C Hamano] Merge branch 'ta/pretty-parse-config' 4740891 8961621 [Junio C Hamano] Merge branch 'bc/archive-pax-header-mode' --- log --decorate --pretty=format:%_d%%h\\ %t\\ [%cn]\\ %s --- (HEAD, upstream/master, master) 85f0837 c29da1d [Junio C Hamano] Start the post-2.1 cycle f655651 4027a43 [Junio C Hamano] Merge branch 'rs/strbuf-getcwd' 51eeaea 1f4970c [Junio C Hamano] Merge branch 'ta/pretty-parse-config' 4740891 8961621 [Junio C Hamano] Merge branch 'bc/archive-pax-header-mode' --- The latter is the output I've been trying to accomplish, and as far as I can tell, this patch is the only way to achieve it. Well, you can do %d%n but that will put a blank line before every commit that doesn't have a ref. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] pretty-format: add append line-feed format specifier
Harry Jeffery ha...@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 '_'. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] pretty-format: add append line-feed format specifier
On Tue, Sep 09, 2014 at 12:37:48PM -0700, Junio C Hamano wrote:
Harry Jeffery ha...@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;
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] pretty-format: add append line-feed format specifier
On 09/09/14 22:45, Jeff King wrote:
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.
I definitely prefer your more general solution to my
bare-minimum-to-scratch-itch patch. I'd certainly be willing to take
your patch and expand upon it (pun unintended) once Junio has weighed in
on your suggestions.
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.
Regardless of what happens with the conditional expansion I think it
would definitely be a useful addition to be able to print the decorators
without the (...) wrapper. I think it's general enough that it'd
warrant its own separate patch rather than being part of a patch series
for the conditional expansion.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] pretty-format: add append line-feed format specifier
On Tue, Sep 09, 2014 at 11:17:20PM +0100, Harry Jeffery wrote: I definitely prefer your more general solution to my bare-minimum-to-scratch-itch patch. I'd certainly be willing to take your patch and expand upon it (pun unintended) once Junio has weighed in on your suggestions. Thanks. I am always happy to see contributors willing to pick up and run with ideas. It is probably out-of-scope for what you want, but while we are talking about %d, it may be worth considering whether there is something simple we can do to make formatting list-like items more flexible. E.g., even with %D, you are stuck with the format foo, bar, baz for multiple decorations. Some kind of %join(%d,; ) might work to produce foo; bar; baz (or whatever you want). But that may also be crossing the line into insanity, and we would be better to allow some Turing-complete embedded language like lua. For that matter, conditionals might be crossing that insanity line, too. Regardless of what happens with the conditional expansion I think it would definitely be a useful addition to be able to print the decorators without the (...) wrapper. I think it's general enough that it'd warrant its own separate patch rather than being part of a patch series for the conditional expansion. Yeah, I agree it can be separate. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
