Re: [PATCH 0/7] config: add --literal-value option

From: Derrick Stolee <stolee@gmail.com>
Date: Nov 20, 2020, 18:51 UTC
Message-ID: <3a347d16-8549-5244-c7db-82b97d7311de@gmail.com>
In-Reply-To: <xmqqk0ufubha.fsf@gitster.c.googlers.com>

On 11/20/2020 1:30 PM, Junio C Hamano wrote:

Show 49 quoted lines

> Derrick Stolee <stolee@gmail.com> writes:
> 
>> On 11/20/2020 8:19 AM, Ævar Arnfjörð Bjarmason wrote:
>>>
>>> On Thu, Nov 19 2020, Derrick Stolee via GitGitGadget wrote:
>>>
>>>> As reported [1], 'git maintenance unregister' fails when a repository is
>>>> located in a directory with regex glob characters.
>>>
>>> Just as bikeshedding on the name: Did you consider something
>>> thematically similar to the corresponding git-grep option,
>>> i.e. --fixed-string[s]. I see -F is also free in git-config(1).
>>
>> I definitely wanted to be specific about "value" in the name,
>> since some options include regexes on the key as well. I'm open
>> to new ideas, and combining your idea with mine would introduce
>> "--fixed-value". Thoughts?
> 
> I very much appreciate "value" is in the name, with the current
> semantics that this only controls how the pattern matching is done
> on the value side and not on the key side.  When making an obvious
> addition of a separate option to control how the pattern matching is
> done on keys in the future, we would regret if we called this option
> "--fixed-strings" today.  And no, I do not think it is an acceptable
> option to introduce "--fixed-strings" that only affects value side
> and then later change its behaviour to affect also on the key side.
> 
> 	Side note.  It _is_ possible to ship such a "--fixed-strings"
> 	option that does not work on the key side and document it as
> 	a known bug, later to be fixed.  I am not sure if I like it.
> 
> But stepping back a bit, is the extra flexibility that allows us to
> control the matching on keys and values separately with such a
> scheme really worth the complexity (at the end-user facing interface
> level, not the implementation level)?
>
> So an alternative may be to use a single option, whose name would
> probably be one of "--(literal|fixed)-(match|strings)", but extend
> the implementation in this series to make the single option affect
> both the value and key matching the same way.
> 
> That would however be more work in the shorter term.  Offhand, I am
> not sure if I like it (i.e. spending time and effort that is more
> than the absolute minimum necessary to fix a breakage.  And the end
> result of doing so is less powerful/flexible, even though it may be
> easier to explain to users simply because the feature is less
> powerful than it could be).  It would be easier, if I can convince
> myself that the extra flexibility is not worth it, to just declare
> that simpler is better here, but I am not quite ready to do so yet.

I had not thought about making this option be related to the key names at all. In particular, we already have the --get-regexp option:

--get-regexp::
	Like --get-all, but interprets the name as a regular expression and
	writes out the key names.  Regular expression matching is currently
	case-sensitive and done against a canonicalized version of the key
	in which section and variable names are lowercased, but subsection
	names are not.

I suppose that there could be reason to create a similar --unset-regexp that is the same equivalent of --unset-all. Perhaps the --replace-all mode could benefit, too? These are also more dangerous to use with regex matches since they also _change_ the config, not just query it.

Since there is already asymmetry between the key and value (only one mode, --get-regexp, has a "name_regex"), I would prefer to treat the value_regex in isolation here.

Thanks, -Stolee