Re: [PATCH 9/9] send-email: move trivial config handling to Perl

From: Jeff King <peff@peff.net>
Date: May 12, 2021, 23:08 UTC
Message-ID: <YJxgC/PJ/JIcIOfq@coredump.intra.peff.net>
In-Reply-To: <patch-9.9-0d87c9a5a3-20210512T132955Z-avarab@gmail.com>

On Wed, May 12, 2021 at 03:48:25PM +0200, Ævar Arnfjörð Bjarmason wrote:

Show 9 quoted lines

> Optimize the startup time of git-send-email by using an amended
> config_regexp() function to retrieve the list of config keys and
> values we're interested in.
> 
> For boolean keys we can handle the [true|false] case ourselves, and
> the "--get" case didn't need any parsing. Let's leave "--path" and
> other "--bool" cases to "git config". As noted in a preceding commit
> we're free to change the config_regexp() function, it's only used by
> "git send-email".

I think both of these cases should be safe.

It is a bit unfortunate to have to go through these contortions, but this is definitely the best we can do for now. I think in the long run it would be nice to have a "--stdin" mode for git-config, where we could do something like:

  git config --stdin <<\EOF
  key=foo.bar
  type=bool
  default=false

  key=another.key
  type=color
  default=red
  EOF

But that doesn't exist yet, and using it would probably involve rearranging send-email a bit (we would need an up-front list of all of the keys we care about and their types). So I'm perfectly content with this strategy in the meantime.

Show 11 quoted lines

> diff --git a/perl/Git.pm b/perl/Git.pm
> index f18852fb09..a9020d0d01 100644
> --- a/perl/Git.pm
> +++ b/perl/Git.pm
> @@ -739,18 +739,18 @@ sub config_int {
>  =item config_regexp ( RE )
>  
>  Retrieve the list of configuration key names matching the regular
> -expression C<RE>. The return value is a list of strings matching
> -this regex.
> +expression C<RE>. The return value is an ARRAY of key-value pairs.

The Git.pm interface is public-facing, isn't it? Are we breaking third-party callers of config_regexp here?

-Peff