git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: "René Scharfe" <l.s.r@web.de>
Cc: "Étienne SERVAIS" <etienne.servais@voucoux.fr>,
	git@vger.kernel.org, "Rafael Ascensão" <rafa.almas@gmail.com>
Subject: Re: Simplify-by-decoration with decorate-refs-exclude
Date: Fri, 02 Aug 2019 12:14:33 -0700	[thread overview]
Message-ID: <xmqq36ijl6qu.fsf@gitster-ct.c.googlers.com> (raw)
In-Reply-To: <37283d4e-3f79-a6b1-425a-f90704fbcce2@web.de> ("René Scharfe"'s message of "Fri, 2 Aug 2019 18:52:51 +0200")

René Scharfe <l.s.r@web.de> 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@voucoux.fr>
> Signed-off-by: René Scharfe <l.s.r@web.de>
> ---
>  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

  reply	other threads:[~2019-08-02 19:14 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-02  8:47 Simplify-by-decoration with decorate-refs-exclude Étienne SERVAIS
2019-08-02 16:52 ` René Scharfe
2019-08-02 19:14   ` Junio C Hamano [this message]
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é Scharfe
2019-08-03 14:44           ` Junio C Hamano
2019-09-08 17:58             ` [PATCH 1/2] log: test --decorate-refs-exclude with --simplify-by-decoration René Scharfe
2019-09-09 18:17               ` Junio C Hamano
2019-09-08 17:58             ` [PATCH 2/2] log-tree: call load_ref_decorations() in get_name_decoration() René Scharfe

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

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqq36ijl6qu.fsf@gitster-ct.c.googlers.com \
    --to=gitster@pobox.com \
    --cc=etienne.servais@voucoux.fr \
    --cc=git@vger.kernel.org \
    --cc=l.s.r@web.de \
    --cc=rafa.almas@gmail.com \
    /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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).