Message ID | pull.796.git.1605801143.gitgitgadget@gmail.com (mailing list archive) |
---|---|
Headers | show |
Series | config: add --literal-value option | expand |
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).
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? Thanks, -Stolee
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. As to "-F", I do not think it is a good idea; in some context "-F" means work on a <file> given via that option, i.e. "-F <file>". Thanks.
On 11/20/2020 1:30 PM, Junio C Hamano wrote: > 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
Derrick Stolee <stolee@gmail.com> writes: > 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. Ah, I forgot about --get-regexp vs --get; so the ship has already sailed---I agree with you that --(fixed|literal)-value is all we need. Thanks.
On Fri, Nov 20 2020, Derrick Stolee wrote: > 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? Hi. I see you've already sent a v2 with that. Looks good to me. FWIW I didn't feel strongly one way or the other about it, either would be a fine choice by someone who's eyeballing this more than my cursory glance. I just wanted to point out the grep option in case you'd missed it, since we've got a bazillion CLI options all over the place, and sometimes there's an existing option that's thematically similar to a proposed new one that's easy to miss. So we can re-use (or partially re-use) the name, behavior, or not.