*Re: Simplify-by-decoration with decorate-refs-exclude
2019-08-02 19:14 ` Junio C Hamano@ 2019-08-02 20:36 ` René Scharfe
2019-08-02 21:20 ` Junio C Hamano0 siblings, 1 reply; 11+ messages in thread
From: René Scharfe @ 2019-08-02 20:36 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Étienne SERVAIS, git, Rafael Ascensão
Am 02.08.19 um 21:14 schrieb Junio C Hamano:
> I can see how this would help, but it somehow feels a bit brittle
> to rely on where the decorations get loaded.
Right.
> I wonder if it would help to move the ability to handle decoration
> filter down from the log layer to revisions.c API layer.
>
> It looks to me that this caller of setup_revisions() can prepare
> decoration_filter before it calls setup_revisions(); we can let the
> revisions.c layer call load_ref_decorations() in setup_revisions()
> if that is the case, no?
Having cmd_log_init_finish() call load_ref_decorations() before
setup_revisions() would indeed solve the issue as well. But we need
to call the latter to check if --pretty=raw was given and avoid loading
decorations in that case, don't we?
> Other two callers of load_ref_decorations() are deep inside pretty.c
> but I wonder in the longer term if we would want to turn them into
> an "a lot higher level should have already loaded decorations"
> assert.
This would require that higher level to parse the user format to check
if %d or %D is present before formatting the first item. Hmm.
René
^permalinkrawreply [flat|nested] 11+ messages in thread

*Re: Simplify-by-decoration with decorate-refs-exclude
2019-08-02 20:36 ` René Scharfe@ 2019-08-02 21:20 ` Junio C Hamano
2019-08-02 23:21 ` Jeff King
2019-08-03 6:51 ` René Scharfe0 siblings, 2 replies; 11+ messages in thread
From: Junio C Hamano @ 2019-08-02 21:20 UTC (permalink / raw)
To: René Scharfe; +Cc: Étienne SERVAIS, git, Rafael Ascensão
René Scharfe <l.s.r@web.de> writes:
> Having cmd_log_init_finish() call load_ref_decorations() before
> setup_revisions() would indeed solve the issue as well. But we need
> to call the latter to check if --pretty=raw was given and avoid loading
> decorations in that case, don't we?
I was thinking about giving an instance of the decoration_filter to
either rev_info or setup_revision_opt, and moving the call to
load_ref_decorations() and the decision to make that call from
cmd_log_init_finish() to setup_revisions().
>> Other two callers of load_ref_decorations() are deep inside pretty.c
>> but I wonder in the longer term if we would want to turn them into
>> an "a lot higher level should have already loaded decorations"
>> assert.
>
> This would require that higher level to parse the user format to check
> if %d or %D is present before formatting the first item. Hmm.
Yes. Don't we pre-scan what kind of formatting primitives are used
in the end-user supplied string already to optimize loading of notes
and source information?
^permalinkrawreply [flat|nested] 11+ messages in thread

*Re: Simplify-by-decoration with decorate-refs-exclude
2019-08-02 21:20 ` Junio C Hamano@ 2019-08-02 23:21 ` Jeff King
2019-08-03 6:51 ` René Scharfe1 sibling, 0 replies; 11+ messages in thread
From: Jeff King @ 2019-08-02 23:21 UTC (permalink / raw)
To: Junio C Hamano
Cc: René Scharfe, Étienne SERVAIS, git, Rafael Ascensão
On Fri, Aug 02, 2019 at 02:20:31PM -0700, Junio C Hamano wrote:
> > This would require that higher level to parse the user format to check
> > if %d or %D is present before formatting the first item. Hmm.
>
> Yes. Don't we pre-scan what kind of formatting primitives are used
> in the end-user supplied string already to optimize loading of notes
> and source information?
I think userformat_find_requirements() is what you're looking for.
I do think it might be tricky to find all of the callers who need to use
it, though. Some of them are not necessarily users of the traversal
machinery, but just have a one-off pretty_print_context.
-Peff
^permalinkrawreply [flat|nested] 11+ messages in thread