On Fri, May 03, 2024 at 06:08:57AM -0700, Karthik Nayak wrote:
> Patrick Steinhardt <ps@xxxxxx> writes:
>
> > While git-config(1) has several modes, those modes are not exposed with
> > subcommands but instead by specifying e.g. `--unset` or `--list`. This
>
> s/specifying/specifying flags/ perhaps?
>
> > user interface is not really in line with how our more modern commands
> > work, where it is a lot more customary to say e.g. `git remote list`.
>
> Tangent: I totally agree with the patch, but it would be nice to have a
> 'DesigningCommands' document which would highlight UX do's and don'ts.
> It would be nice to add that as reference in discussions.
I agree that we should have that discussion, but feel like it should be
part of a separate patch series. If I were to adopt that into this
series I very much think that the resulting discussions would take quite
a while to settle.
I'll start the discussion soonish in a separate thread.
[snip]
> > @@ -2590,7 +2608,7 @@ test_expect_success '--fixed-value uses exact string matching' '
> > test_cmp expect actual &&
> >
> > git config --file=config --fixed-value --replace-all fixed.test bogus "$META" &&
> > - git config --file=config --list >actual &&
> > + git config ${mode_prefix}list --file=config >actual &&
> > cat >expect <<-EOF &&
> > fixed.test=bogus
> > fixed.test=bogus
> > @@ -2751,4 +2769,6 @@ test_expect_success 'specifying multiple modes causes failure' '
> > test_cmp expect err
> > '
> >
> > +done
> > +
> >
> Nit: Wouldn't it be better if the tests are indented here? That way you
> know it's part of a loop.
We would basically have to reindent 2500 lines of code. I don't think
that'd be a helpful to reviewers :)
Patrick
Attachment:
signature.asc
Description: PGP signature
