René Scharfe <l.s.r@xxxxxx> writes:
> Am 02.08.19 um 10:47 schrieb Étienne SERVAIS:
>> Thus, when I enter
>>
>> ```
>> git log --oneline --graph --decorate=full --decorate-refs-exclude='refs/tags/<pattern>'
>> ```
>> The selected tags are properly excluded but once I add the
>> `simplify-by-decoration` option
>>
>> ```
>> git log --oneline --graph --decorate=full --decorate-refs-exclude='refs/tags/<pattern>' --simplify-by-decoration
>> ```
>> The excluded tags pop back again.
>
> Does this help?
I can see how this would help, but it somehow feels a bit brittle
to rely on where the decorations get loaded.
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?
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.
Thanks.
> -- >8 --
> Subject: [PATCH] revision: load decorations lazily for --simplify-by-decoration
>
> Let setup_revisions() and friends respect a filtered set of decoration
> refs loaded by callers by postponing its own load_ref_decorations() call
> to just before decorations are used to simplify history. That function
> only does any actual work the first time it is called.
>
> This allows using the revision option --simplify-by-decoration together
> with the log option --decorate-refs-exclude and having it simplify over
> the restricted set of refs.
>
> Reported-by: Étienne SERVAIS <etienne.servais@xxxxxxxxxx>
> Signed-off-by: René Scharfe <l.s.r@xxxxxx>
> ---
> revision.c | 8 +++++++-
> t/t4202-log.sh | 15 +++++++++++++++
> 2 files changed, 22 insertions(+), 1 deletion(-)
>
> diff --git a/revision.c b/revision.c
> index 07412297f0..d3456c959b 100644
> --- a/revision.c
> +++ b/revision.c
> @@ -633,6 +633,13 @@ static int rev_compare_tree(struct rev_info *revs,
> return REV_TREE_OLD;
>
> if (revs->simplify_by_decoration) {
> + /*
> + * Load decorations lazily; later calls have no effect.
> + * This gives callers a chance to load a restricted set
> + * beforehand.
> + */
> + load_ref_decorations(NULL, DECORATE_SHORT_REFS);
> +
> /*
> * If we are simplifying by decoration, then the commit
> * is worth showing if it has a tag pointing at it.
> @@ -2063,7 +2070,6 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
> revs->simplify_by_decoration = 1;
> revs->limited = 1;
> revs->prune = 1;
> - load_ref_decorations(NULL, DECORATE_SHORT_REFS);
> } else if (!strcmp(arg, "--date-order")) {
> revs->sort_order = REV_SORT_BY_COMMIT_DATE;
> revs->topo_order = 1;
> diff --git a/t/t4202-log.sh b/t/t4202-log.sh
> index c20209324c..bb66d1d93c 100755
> --- a/t/t4202-log.sh
> +++ b/t/t4202-log.sh
> @@ -837,6 +837,21 @@ test_expect_success 'decorate-refs and decorate-refs-exclude' '
> test_cmp expect.decorate actual
> '
>
> +test_expect_success 'decorate-refs-exclude and simplify-by-decoration' '
> + cat >expect.decorate <<-\EOF &&
> + Merge-tag-reach (HEAD -> master)
> + reach (tag: reach, reach)
> + seventh (tag: seventh)
> + Merge-branch-tangle
> + Merge-branch-side-early-part-into-tangle (tangle)
> + tangle-a (tag: tangle-a)
> + EOF
> + git log -n6 --decorate=short --pretty="tformat:%f%d" \
> + --decorate-refs-exclude="*octopus*" \
> + --simplify-by-decoration >actual &&
> + test_cmp expect.decorate actual
> +'
> +
> test_expect_success 'log.decorate config parsing' '
> git log --oneline --decorate=full >expect.full &&
> git log --oneline --decorate=short >expect.short &&
> --
> 2.22.0
