Re: [PATCH 08/20] config: introduce missing setters that take repo as parameter

From: Justin Tobler <jltobler@gmail.com>
Date: Aug 9, 2024, 20:07 UTC
Message-ID: <rg5j5ehur5a2qvpt65ezfub5dmmqrbn7if5f2r7ibweuupsnm2@qeciml3qvdvy>
In-Reply-To: <feae2ad31ac91baae75c46c22c5c3ef3b58c1897.1723013714.git.ps@pks.im>

On 24/08/07 08:57AM, Patrick Steinhardt wrote:

> While we already provide some of the config-setting interfaces with a
> `struct repository` as parameter, others only have a variant that
> implicitly depend on `the_repository`. Fill in those gaps such that we

s/depend/depends/

Show 18 quoted lines

> can start to deprecate the repo-less variants.
> 
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  config.c | 93 ++++++++++++++++++++++++++++++++++++++++++++------------
>  config.h | 15 ++++++++-
>  2 files changed, 87 insertions(+), 21 deletions(-)
> 
> diff --git a/config.c b/config.c
> index 6421894614..ac89b708e7 100644
> --- a/config.c
> +++ b/config.c
> @@ -3178,21 +3178,39 @@ static void maybe_remove_section(struct config_store_data *store,
>  		*end_offset = store->parsed[store->parsed_nr - 1].end;
>  }
>  
> +int repo_config_set_in_file_gently(struct repository *r, const char *config_filename,
> +				   const char *key, const char *comment, const char *value)

Instead of prefixing with `repo_`, should we instead use the `config_` prefix? Maybe `config_repo_`? It might be nice for names to align with the config subsystem here.

[snip]

Show 57 quoted lines

> diff --git a/config.h b/config.h
> index 54b47dec9e..b13e1bfb8d 100644
> --- a/config.h
> +++ b/config.h
> @@ -298,14 +298,18 @@ int git_config_pathname(char **, const char *, const char *);
>  int git_config_expiry_date(timestamp_t *, const char *, const char *);
>  int git_config_color(char *, const char *, const char *);
>  int git_config_set_in_file_gently(const char *, const char *, const char *, const char *);
> +int repo_config_set_in_file_gently(struct repository *r, const char *config_filename,
> +				   const char *key, const char *comment, const char *value);
>  
>  /**
>   * write config values to a specific config file, takes a key/value pair as
>   * parameter.
>   */
>  void git_config_set_in_file(const char *, const char *, const char *);
> +void repo_config_set_in_file(struct repository *, const char *, const char *, const char *);
>  
>  int git_config_set_gently(const char *, const char *);
> +int repo_config_set_gently(struct repository *r, const char *, const char *);
>  
>  /**
>   * Write a config value that should apply to the current worktree. If
> @@ -318,6 +322,7 @@ int repo_config_set_worktree_gently(struct repository *, const char *, const cha
>   * write config values to `.git/config`, takes a key/value pair as parameter.
>   */
>  void git_config_set(const char *, const char *);
> +void repo_config_set(struct repository *, const char *, const char *);
>  
>  int git_config_parse_key(const char *, char **, size_t *);
>  
> @@ -341,9 +346,11 @@ int git_config_parse_key(const char *, char **, size_t *);
>  #define CONFIG_FLAGS_FIXED_VALUE (1 << 1)
>  
>  int git_config_set_multivar_gently(const char *, const char *, const char *, unsigned);
> -void git_config_set_multivar(const char *, const char *, const char *, unsigned);
>  int repo_config_set_multivar_gently(struct repository *, const char *, const char *, const char *, unsigned);
> +void git_config_set_multivar(const char *, const char *, const char *, unsigned);
> +void repo_config_set_multivar(struct repository *r, const char *, const char *, const char *, unsigned);
>  int git_config_set_multivar_in_file_gently(const char *, const char *, const char *, const char *, const char *, unsigned);
> +int repo_config_set_multivar_in_file_gently(struct repository *, const char *, const char *, const char *, const char *, const char *, unsigned);
>  
>  char *git_config_prepare_comment_string(const char *);
>  
> @@ -372,6 +379,12 @@ void git_config_set_multivar_in_file(const char *config_filename,
>  				     const char *value,
>  				     const char *value_pattern,
>  				     unsigned flags);
> +void repo_config_set_multivar_in_file(struct repository *r,
> +				      const char *config_filename,
> +				      const char *key,
> +				      const char *value,
> +				      const char *value_pattern,
> +				      unsigned flags);
>  
>  /**
>   * rename or remove sections in the config file

The rest of this patch is simply implementing variations of existing functions that explicitly inject a repository and looks good to me.