git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Glen Choo <chooglen@google.com>
To: Jonathan Tan <jonathantanmy@google.com>, git@vger.kernel.org
Cc: Jonathan Tan <jonathantanmy@google.com>, avarab@gmail.com
Subject: Re: [PATCH v6 2/2] config: include file if remote URL matches a glob
Date: Wed, 08 Dec 2021 11:19:33 -0800	[thread overview]
Message-ID: <kl6l35n2c3ay.fsf@chooglen-macbookpro.roam.corp.google.com> (raw)
In-Reply-To: <de2be06818781bbef29926c0e8e8981746242c60.1638919346.git.jonathantanmy@google.com>

Jonathan Tan <jonathantanmy@google.com> writes:

> @@ -335,9 +427,16 @@ static int git_config_include(const char *var, const char *value, void *data)
>  		ret = handle_path_include(value, inc);
>  
>  	if (!parse_config_key(var, "includeif", &cond, &cond_len, &key) &&
> -	    (cond && include_condition_is_true(inc->opts, cond, cond_len)) &&
> -	    !strcmp(key, "path"))
> +	    cond && include_condition_is_true(inc, cond, cond_len) &&
> +	    !strcmp(key, "path")) {
> +		config_fn_t old_fn = inc->fn;
> +
> +		if (inc->opts->unconditional_remote_url)
> +			inc->fn = forbid_remote_url;
>  		ret = handle_path_include(value, inc);
> +		if (inc->opts->unconditional_remote_url)
> +			inc->fn = old_fn;
> +	}
>  
>  	return ret;
>  }

Minor nit: it looks like we don't need to restore inc->fn conditionally,
so instead of:

	if (inc->opts->unconditional_remote_url)
			inc->fn = old_fn;

we could just have:

  inc->fn = old_fn;

which (purely as a matter of personal taste) looks a bit more consistent
with the unconditional assignment of:

  config_fn_t old_fn = inc->fn;



No comments on the rest of the patch; it looks clean and
easy-to-understand :)

  reply	other threads:[~2021-12-08 19:19 UTC|newest]

Thread overview: 87+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-12 22:57 [RFC PATCH 0/2] Conditional config includes based on remote URL Jonathan Tan
2021-10-12 22:57 ` [RFC PATCH 1/2] config: make git_config_include() static Jonathan Tan
2021-10-12 23:07   ` Jeff King
2021-10-12 23:26   ` Junio C Hamano
2021-10-13  8:26   ` Ævar Arnfjörð Bjarmason
2021-10-13 17:00     ` Junio C Hamano
2021-10-13 18:13       ` Jonathan Tan
2021-10-12 22:57 ` [RFC PATCH 2/2] config: include file if remote URL matches a glob Jonathan Tan
2021-10-12 23:30   ` Jeff King
2021-10-13 18:33     ` Jonathan Tan
2021-10-27 11:40       ` Jeff King
2021-10-27 17:23         ` Jonathan Tan
2021-10-12 23:48   ` Junio C Hamano
2021-10-13 19:52     ` Jonathan Tan
2021-10-13  0:46 ` [RFC PATCH 0/2] Conditional config includes based on remote URL brian m. carlson
2021-10-13 18:17   ` Jonathan Tan
2021-10-18 20:48 ` Jonathan Tan
2021-10-22  3:12   ` Emily Shaffer
2021-10-27 11:55   ` Jeff King
2021-10-27 17:52     ` Jonathan Tan
2021-10-27 20:32       ` Jeff King
2021-10-25 13:03 ` Ævar Arnfjörð Bjarmason
2021-10-25 18:53   ` Jonathan Tan
2021-10-26 10:12     ` Ævar Arnfjörð Bjarmason
2021-10-29 17:31 ` [WIP v2 " Jonathan Tan
2021-10-29 17:31   ` [WIP v2 1/2] config: make git_config_include() static Jonathan Tan
2021-11-05 19:45     ` Emily Shaffer
2021-10-29 17:31   ` [WIP v2 2/2] config: include file if remote URL matches a glob Jonathan Tan
2021-11-05 20:24     ` Emily Shaffer
2021-11-06  4:41       ` Ævar Arnfjörð Bjarmason
2021-11-09  0:25         ` Jonathan Tan
2021-11-09  0:22       ` Jonathan Tan
2021-11-16  0:00 ` [PATCH v3 0/2] Conditional config includes based on remote URL Jonathan Tan
2021-11-16  0:00   ` [PATCH v3 1/2] config: make git_config_include() static Jonathan Tan
2021-11-16  0:00   ` [PATCH v3 2/2] config: include file if remote URL matches a glob Jonathan Tan
2021-11-22 22:59     ` Glen Choo
2021-11-29 17:53       ` Jonathan Tan
2021-11-23  1:22     ` Junio C Hamano
2021-11-29 18:18       ` Jonathan Tan
2021-12-01 18:51         ` Junio C Hamano
2021-12-02 23:14           ` Jonathan Tan
2021-11-23  1:27     ` Ævar Arnfjörð Bjarmason
2021-11-29 18:33       ` Jonathan Tan
2021-11-29 20:50         ` Ævar Arnfjörð Bjarmason
2021-11-29 20:23 ` [PATCH v4 0/2] Conditional config includes based on remote URL Jonathan Tan
2021-11-29 20:23   ` [PATCH v4 1/2] config: make git_config_include() static Jonathan Tan
2021-11-29 20:23   ` [PATCH v4 2/2] config: include file if remote URL matches a glob Jonathan Tan
2021-12-02  6:57     ` Junio C Hamano
2021-12-02 17:41       ` Jonathan Tan
2021-11-29 20:48   ` [PATCH v4 0/2] Conditional config includes based on remote URL Ævar Arnfjörð Bjarmason
2021-11-30  7:51     ` Junio C Hamano
2021-12-02 23:31 ` [PATCH v5 " Jonathan Tan
2021-12-02 23:31   ` [PATCH v5 1/2] config: make git_config_include() static Jonathan Tan
2021-12-02 23:31   ` [PATCH v5 2/2] config: include file if remote URL matches a glob Jonathan Tan
2021-12-06 22:32     ` Glen Choo
2021-12-07 17:53       ` Jonathan Tan
2021-12-06 18:57   ` [PATCH v5 0/2] Conditional config includes based on remote URL Ævar Arnfjörð Bjarmason
2021-12-07 17:46     ` Jonathan Tan
2021-12-07 17:56       ` Ævar Arnfjörð Bjarmason
2021-12-07 18:52         ` Jonathan Tan
2021-12-07 23:23 ` [PATCH v6 " Jonathan Tan
2021-12-07 23:23   ` [PATCH v6 1/2] config: make git_config_include() static Jonathan Tan
2021-12-07 23:23   ` [PATCH v6 2/2] config: include file if remote URL matches a glob Jonathan Tan
2021-12-08 19:19     ` Glen Choo [this message]
2021-12-09 22:16       ` Jonathan Tan
2021-12-08 19:55     ` Glen Choo
2021-12-09 22:39       ` Jonathan Tan
2021-12-09 23:33         ` Glen Choo
2021-12-13 23:35           ` Jonathan Tan
2021-12-10 21:45         ` Junio C Hamano
2021-12-13 23:37           ` Jonathan Tan
2021-12-14 21:31 ` [PATCH v7 0/2] Conditional config includes based on remote URL Jonathan Tan
2021-12-14 21:31   ` [PATCH v7 1/2] config: make git_config_include() static Jonathan Tan
2021-12-14 21:31   ` [PATCH v7 2/2] config: include file if remote URL matches a glob Jonathan Tan
2021-12-16 21:54     ` Glen Choo
2021-12-28  0:55     ` Elijah Newren
2022-01-10 18:58       ` Jonathan Tan
2021-12-16 21:57   ` [PATCH v7 0/2] Conditional config includes based on remote URL Glen Choo
2021-12-28  1:13   ` Elijah Newren
2021-12-28 23:13     ` Glen Choo
2022-01-10 19:22     ` Jonathan Tan
2022-01-10 20:17       ` Elijah Newren
2022-01-25 13:26         ` Scalar vs JGit, was " Johannes Schindelin
2022-01-18 17:47 ` [PATCH v8 " Jonathan Tan
2022-01-18 17:47   ` [PATCH v8 1/2] config: make git_config_include() static Jonathan Tan
2022-01-18 17:47   ` [PATCH v8 2/2] config: include file if remote URL matches a glob Jonathan Tan
2022-01-18 20:54   ` [PATCH v8 0/2] Conditional config includes based on remote URL Elijah Newren

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=kl6l35n2c3ay.fsf@chooglen-macbookpro.roam.corp.google.com \
    --to=chooglen@google.com \
    --cc=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=jonathantanmy@google.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).