From: Junio C Hamano <gitster@pobox.com>
To: "Glen Choo via GitGitGadget" <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org, "Jonathan Tan" <jonathantanmy@google.com>,
"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
"Emily Shaffer" <nasamuffin@google.com>,
"Phillip Wood" <phillip.wood123@gmail.com>,
"Glen Choo" <chooglen@google.com>
Subject: Re: [PATCH v3 01/12] config: inline git_color_default_config
Date: Tue, 20 Jun 2023 14:01:45 -0700 [thread overview]
Message-ID: <xmqqjzvxlufa.fsf@gitster.g> (raw)
In-Reply-To: <26109b65142d2a325201940262a6c0bf183ec4bd.1687290232.git.gitgitgadget@gmail.com> (Glen Choo via GitGitGadget's message of "Tue, 20 Jun 2023 19:43:40 +0000")
"Glen Choo via GitGitGadget" <gitgitgadget@gmail.com> writes:
> From: Glen Choo <chooglen@google.com>
>
> git_color_default_config() is a shorthand for calling two other config
> callbacks. There are no other non-static functions that do this and it
> will complicate our refactoring of config_fn_t so inline it instead.
>
> Signed-off-by: Glen Choo <chooglen@google.com>
> ---
We would need to deal with git_default_config() that is called from
everywhere in exactly the same way, so I am not sure if the presence
of git_color_default_config() will complicate so much to require
this step while git_default_config() can be dealt with without
inlining into its callers, but this change does not hurt anybody
either.
OK.
> builtin/branch.c | 5 ++++-
> builtin/clean.c | 6 ++++--
> builtin/grep.c | 5 ++++-
> builtin/show-branch.c | 5 ++++-
> builtin/tag.c | 6 +++++-
> color.c | 8 --------
> color.h | 6 +-----
> 7 files changed, 22 insertions(+), 19 deletions(-)
>
> diff --git a/builtin/branch.c b/builtin/branch.c
> index e6c2655af68..df99e38847b 100644
> --- a/builtin/branch.c
> +++ b/builtin/branch.c
> @@ -117,7 +117,10 @@ static int git_branch_config(const char *var, const char *value, void *cb)
> return 0;
> }
>
> - return git_color_default_config(var, value, cb);
> + if (git_color_config(var, value, cb) < 0)
> + return -1;
> +
> + return git_default_config(var, value, cb);
> }
>
> static const char *branch_get_color(enum color_branch ix)
> diff --git a/builtin/clean.c b/builtin/clean.c
> index 78852d28cec..57e7f7cac64 100644
> --- a/builtin/clean.c
> +++ b/builtin/clean.c
> @@ -130,8 +130,10 @@ static int git_clean_config(const char *var, const char *value, void *cb)
> return 0;
> }
>
> - /* inspect the color.ui config variable and others */
> - return git_color_default_config(var, value, cb);
> + if (git_color_config(var, value, cb) < 0)
> + return -1;
> +
> + return git_default_config(var, value, cb);
> }
>
> static const char *clean_get_color(enum color_clean ix)
> diff --git a/builtin/grep.c b/builtin/grep.c
> index b86c754defb..76cf999d310 100644
> --- a/builtin/grep.c
> +++ b/builtin/grep.c
> @@ -293,7 +293,10 @@ static int wait_all(void)
> static int grep_cmd_config(const char *var, const char *value, void *cb)
> {
> int st = grep_config(var, value, cb);
> - if (git_color_default_config(var, value, NULL) < 0)
> +
> + if (git_color_config(var, value, cb) < 0)
> + st = -1;
> + else if (git_default_config(var, value, cb) < 0)
> st = -1;
>
> if (!strcmp(var, "grep.threads")) {
> diff --git a/builtin/show-branch.c b/builtin/show-branch.c
> index 7ef4a642c17..a2461270d4b 100644
> --- a/builtin/show-branch.c
> +++ b/builtin/show-branch.c
> @@ -579,7 +579,10 @@ static int git_show_branch_config(const char *var, const char *value, void *cb)
> return 0;
> }
>
> - return git_color_default_config(var, value, cb);
> + if (git_color_config(var, value, cb) < 0)
> + return -1;
> +
> + return git_default_config(var, value, cb);
> }
>
> static int omit_in_dense(struct commit *commit, struct commit **rev, int n)
> diff --git a/builtin/tag.c b/builtin/tag.c
> index 49b64c7a288..1acf5f7a59f 100644
> --- a/builtin/tag.c
> +++ b/builtin/tag.c
> @@ -209,7 +209,11 @@ static int git_tag_config(const char *var, const char *value, void *cb)
>
> if (starts_with(var, "column."))
> return git_column_config(var, value, "tag", &colopts);
> - return git_color_default_config(var, value, cb);
> +
> + if (git_color_config(var, value, cb) < 0)
> + return -1;
> +
> + return git_default_config(var, value, cb);
> }
>
> static void write_tag_body(int fd, const struct object_id *oid)
> diff --git a/color.c b/color.c
> index 83abb11eda0..b24b19566b9 100644
> --- a/color.c
> +++ b/color.c
> @@ -430,14 +430,6 @@ int git_color_config(const char *var, const char *value, void *cb UNUSED)
> return 0;
> }
>
> -int git_color_default_config(const char *var, const char *value, void *cb)
> -{
> - if (git_color_config(var, value, cb) < 0)
> - return -1;
> -
> - return git_default_config(var, value, cb);
> -}
> -
> void color_print_strbuf(FILE *fp, const char *color, const struct strbuf *sb)
> {
> if (*color)
> diff --git a/color.h b/color.h
> index cfc8f841b23..bb28343be21 100644
> --- a/color.h
> +++ b/color.h
> @@ -88,12 +88,8 @@ extern const int column_colors_ansi_max;
> */
> extern int color_stdout_is_tty;
>
> -/*
> - * Use the first one if you need only color config; the second is a convenience
> - * if you are just going to change to git_default_config, too.
> - */
> +/* Parse color config. */
> int git_color_config(const char *var, const char *value, void *cb);
> -int git_color_default_config(const char *var, const char *value, void *cb);
>
> /*
> * Parse a config option, which can be a boolean or one of
next prev parent reply other threads:[~2023-06-20 21:01 UTC|newest]
Thread overview: 115+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-04-21 19:13 [PATCH 00/14] [RFC] config: remove global state from config iteration Glen Choo via GitGitGadget
2023-04-21 19:13 ` [PATCH 01/14] config.c: introduce kvi_fn(), use it for configsets Glen Choo via GitGitGadget
2023-04-21 19:13 ` [PATCH 02/14] config.c: use kvi for CLI config Glen Choo via GitGitGadget
2023-05-01 11:06 ` Ævar Arnfjörð Bjarmason
2023-04-21 19:13 ` [PATCH 03/14] config: use kvi for config files Glen Choo via GitGitGadget
2023-04-21 19:13 ` [PATCH 04/14] config: add kvi.path, use it to evaluate includes Glen Choo via GitGitGadget
2023-04-21 19:13 ` [PATCH 05/14] config: pass source to config_parser_event_fn_t Glen Choo via GitGitGadget
2023-04-21 19:13 ` [PATCH 06/14] config: inline git_color_default_config Glen Choo via GitGitGadget
2023-04-21 19:13 ` [PATCH 07/14] urlmatch.h: use config_fn_t type Glen Choo via GitGitGadget
2023-04-21 19:13 ` [PATCH 08/14] (RFC-only) config: add kvi arg to config_fn_t Glen Choo via GitGitGadget
2023-04-21 19:13 ` [PATCH 09/14] (RFC-only) config: apply cocci to config_fn_t implementations Glen Choo via GitGitGadget
2023-04-21 19:13 ` [PATCH 10/14] (RFC-only) config: finish config_fn_t refactor Glen Choo via GitGitGadget
2023-05-01 11:19 ` Ævar Arnfjörð Bjarmason
2023-05-05 21:07 ` Jonathan Tan
2023-05-09 22:46 ` Glen Choo
2023-05-11 16:21 ` Jonathan Tan
2023-05-08 21:00 ` Glen Choo
2023-04-21 19:13 ` [PATCH 11/14] config: remove current_config_(line|name|origin_type) Glen Choo via GitGitGadget
2023-04-21 19:13 ` [PATCH 12/14] config: remove current_config_scope() Glen Choo via GitGitGadget
2023-04-21 19:13 ` [PATCH 13/14] config: pass kvi to die_bad_number() Glen Choo via GitGitGadget
2023-04-21 19:13 ` [PATCH 14/14] config: remove config_reader from configset_add_value Glen Choo via GitGitGadget
2023-05-30 18:41 ` [PATCH v2 00/14] [RFC] config: remove global state from config iteration Glen Choo via GitGitGadget
2023-05-30 18:41 ` [PATCH v2 01/14] config: inline git_color_default_config Glen Choo via GitGitGadget
2023-05-30 18:42 ` [PATCH v2 02/14] urlmatch.h: use config_fn_t type Glen Choo via GitGitGadget
2023-05-30 18:42 ` [PATCH v2 03/14] (RFC-only) config: add kvi arg to config_fn_t Glen Choo via GitGitGadget
2023-06-01 9:50 ` Phillip Wood
2023-06-01 16:22 ` Glen Choo
2023-06-02 9:54 ` Phillip Wood
2023-06-02 16:46 ` Glen Choo
2023-06-05 9:38 ` Phillip Wood
2023-06-09 23:19 ` Glen Choo
2023-05-30 18:42 ` [PATCH v2 04/14] (RFC-only) config: apply cocci to config_fn_t implementations Glen Choo via GitGitGadget
2023-05-30 18:42 ` [PATCH v2 05/14] (RFC-only) config: finish config_fn_t refactor Glen Choo via GitGitGadget
2023-06-01 22:17 ` Jonathan Tan
2023-05-30 18:42 ` [PATCH v2 06/14] config.c: pass kvi in configsets Glen Choo via GitGitGadget
2023-06-01 22:21 ` Jonathan Tan
2023-05-30 18:42 ` [PATCH v2 07/14] config: provide kvi with config files Glen Choo via GitGitGadget
2023-06-01 22:41 ` Jonathan Tan
2023-06-01 23:54 ` Jonathan Tan
2023-05-30 18:42 ` [PATCH v2 08/14] builtin/config.c: test misuse of format_config() Glen Choo via GitGitGadget
2023-05-30 18:42 ` [PATCH v2 09/14] config.c: provide kvi with CLI config Glen Choo via GitGitGadget
2023-06-01 23:35 ` Jonathan Tan
2023-06-02 17:26 ` Glen Choo
2023-05-30 18:42 ` [PATCH v2 10/14] trace2: plumb config kvi Glen Choo via GitGitGadget
2023-06-01 23:38 ` Jonathan Tan
2023-05-30 18:42 ` [PATCH v2 11/14] config: pass kvi to die_bad_number() Glen Choo via GitGitGadget
2023-06-01 23:48 ` Jonathan Tan
2023-06-02 17:23 ` Glen Choo
2023-05-30 18:42 ` [PATCH v2 12/14] config.c: remove config_reader from configsets Glen Choo via GitGitGadget
2023-05-30 18:42 ` [PATCH v2 13/14] config: add kvi.path, use it to evaluate includes Glen Choo via GitGitGadget
2023-06-02 0:06 ` Jonathan Tan
2023-05-30 18:42 ` [PATCH v2 14/14] config: pass source to config_parser_event_fn_t Glen Choo via GitGitGadget
2023-06-02 0:08 ` Jonathan Tan
2023-06-02 17:20 ` Glen Choo
2023-06-20 19:43 ` [PATCH v3 00/12] config: remove global state from config iteration Glen Choo via GitGitGadget
2023-06-20 19:43 ` [PATCH v3 01/12] config: inline git_color_default_config Glen Choo via GitGitGadget
2023-06-20 21:01 ` Junio C Hamano [this message]
2023-06-20 19:43 ` [PATCH v3 02/12] urlmatch.h: use config_fn_t type Glen Choo via GitGitGadget
2023-06-20 21:02 ` Junio C Hamano
2023-06-20 19:43 ` [PATCH v3 03/12] config: add ctx arg to config_fn_t Glen Choo via GitGitGadget
2023-06-20 19:43 ` [PATCH v3 04/12] config.c: pass ctx in configsets Glen Choo via GitGitGadget
2023-06-20 21:19 ` Junio C Hamano
2023-06-20 19:43 ` [PATCH v3 05/12] config: pass ctx with config files Glen Choo via GitGitGadget
2023-06-20 19:43 ` [PATCH v3 06/12] builtin/config.c: test misuse of format_config() Glen Choo via GitGitGadget
2023-06-20 21:35 ` Junio C Hamano
2023-06-20 23:06 ` Glen Choo
2023-06-23 20:32 ` Jonathan Tan
2023-06-24 1:31 ` Jeff King
2023-06-28 17:28 ` Glen Choo
2023-06-20 19:43 ` [PATCH v3 07/12] config.c: pass ctx with CLI config Glen Choo via GitGitGadget
2023-06-23 20:35 ` Jonathan Tan
2023-06-23 21:41 ` Glen Choo
2023-06-20 19:43 ` [PATCH v3 08/12] trace2: plumb config kvi Glen Choo via GitGitGadget
2023-06-23 20:40 ` Jonathan Tan
2023-06-20 19:43 ` [PATCH v3 09/12] config: pass kvi to die_bad_number() Glen Choo via GitGitGadget
2023-06-20 19:43 ` [PATCH v3 10/12] config.c: remove config_reader from configsets Glen Choo via GitGitGadget
2023-06-23 20:57 ` Jonathan Tan
2023-06-23 21:33 ` Junio C Hamano
2023-06-20 19:43 ` [PATCH v3 11/12] config: add kvi.path, use it to evaluate includes Glen Choo via GitGitGadget
2023-06-20 19:43 ` [PATCH v3 12/12] config: pass source to config_parser_event_fn_t Glen Choo via GitGitGadget
2023-06-20 21:46 ` Junio C Hamano
2023-06-21 21:46 ` [PATCH v3 00/12] config: remove global state from config iteration Junio C Hamano
2023-06-21 23:06 ` Glen Choo
2023-06-23 21:02 ` Jonathan Tan
2023-06-23 21:33 ` Junio C Hamano
2023-06-23 21:45 ` Glen Choo
2023-06-26 18:11 ` [PATCH v4 " Glen Choo via GitGitGadget
2023-06-26 18:11 ` [PATCH v4 01/12] config: inline git_color_default_config Glen Choo via GitGitGadget
2023-06-26 18:11 ` [PATCH v4 02/12] urlmatch.h: use config_fn_t type Glen Choo via GitGitGadget
2023-06-26 18:11 ` [PATCH v4 03/12] config: add ctx arg to config_fn_t Glen Choo via GitGitGadget
2023-06-26 18:11 ` [PATCH v4 04/12] config.c: pass ctx in configsets Glen Choo via GitGitGadget
2023-06-26 18:11 ` [PATCH v4 05/12] config: pass ctx with config files Glen Choo via GitGitGadget
2023-06-26 18:11 ` [PATCH v4 06/12] builtin/config.c: test misuse of format_config() Glen Choo via GitGitGadget
2023-06-26 18:11 ` [PATCH v4 07/12] config.c: pass ctx with CLI config Glen Choo via GitGitGadget
2023-06-26 18:11 ` [PATCH v4 08/12] trace2: plumb config kvi Glen Choo via GitGitGadget
2023-06-26 18:11 ` [PATCH v4 09/12] config: pass kvi to die_bad_number() Glen Choo via GitGitGadget
2023-06-26 18:11 ` [PATCH v4 10/12] config.c: remove config_reader from configsets Glen Choo via GitGitGadget
2023-06-26 18:11 ` [PATCH v4 11/12] config: add kvi.path, use it to evaluate includes Glen Choo via GitGitGadget
2023-06-26 18:11 ` [PATCH v4 12/12] config: pass source to config_parser_event_fn_t Glen Choo via GitGitGadget
2023-06-26 20:45 ` [PATCH v4 00/12] config: remove global state from config iteration Junio C Hamano
2023-06-28 19:26 ` [PATCH v5 00/11] " Glen Choo via GitGitGadget
2023-06-28 19:26 ` [PATCH v5 01/11] config: inline git_color_default_config Glen Choo via GitGitGadget
2023-06-28 19:26 ` [PATCH v5 02/11] urlmatch.h: use config_fn_t type Glen Choo via GitGitGadget
2023-06-28 19:26 ` [PATCH v5 03/11] config: add ctx arg to config_fn_t Glen Choo via GitGitGadget
2023-06-28 19:26 ` [PATCH v5 04/11] config.c: pass ctx in configsets Glen Choo via GitGitGadget
2023-06-28 19:26 ` [PATCH v5 05/11] config: pass ctx with config files Glen Choo via GitGitGadget
2023-06-28 19:26 ` [PATCH v5 06/11] config.c: pass ctx with CLI config Glen Choo via GitGitGadget
2023-06-28 19:26 ` [PATCH v5 07/11] trace2: plumb config kvi Glen Choo via GitGitGadget
2023-06-28 19:26 ` [PATCH v5 08/11] config: pass kvi to die_bad_number() Glen Choo via GitGitGadget
2023-06-28 19:26 ` [PATCH v5 09/11] config.c: remove config_reader from configsets Glen Choo via GitGitGadget
2023-06-28 19:26 ` [PATCH v5 10/11] config: add kvi.path, use it to evaluate includes Glen Choo via GitGitGadget
2023-06-28 19:26 ` [PATCH v5 11/11] config: pass source to config_parser_event_fn_t Glen Choo via GitGitGadget
2023-06-28 22:23 ` [PATCH v5 00/11] config: remove global state from config iteration Jonathan Tan
2023-06-28 22:47 ` Junio C Hamano
2023-07-11 18:41 ` Phillip Wood
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
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqjzvxlufa.fsf@gitster.g \
--to=gitster@pobox.com \
--cc=avarab@gmail.com \
--cc=chooglen@google.com \
--cc=git@vger.kernel.org \
--cc=gitgitgadget@gmail.com \
--cc=jonathantanmy@google.com \
--cc=nasamuffin@google.com \
--cc=phillip.wood123@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.
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).