On Mon, Mar 26, 2018 at 04:34:42AM -0400, Jeff King wrote: > > diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt > > index e09ed5d7d..d9e389a33 100644 > > --- a/Documentation/git-config.txt > > +++ b/Documentation/git-config.txt > > @@ -233,8 +233,10 @@ See also <<FILES>>. > > using `--file`, `--global`, etc) and `on` when searching all > > config files. > > > > -CONFIGURATION > > -------------- > > +--default value:: > > + When using `--get`, behave as if value were the value assigned to the given > > + slot. > > + > > `pager.config` is only respected when listing configuration, i.e., when > > using `--list` or any of the `--get-*` which may return multiple results. > > The default is to use a pager. > > Hmm, what's going on with the CONFIGURATION header here? My mistake, I have correct this erroneous diff in v3. Thanks for pointing it out! > For the description of "--default" itself, do we want to say something > like: > > When using `--get` and the requested slot is not found, behave as > if... > > That is hopefully a given from the name "--default", but it seems like > an important point to mention. Ditto. > > +test_expect_success 'marshals default value as int' ' > > + echo 810 >expect && > > + git config --default 810 --int core.foo >actual && > > + test_cmp expect actual > > +' > > + > > +test_expect_success 'marshals default value as int (storage unit)' ' > > + echo 1048576 >expect && > > + git config --default 1M --int core.foo >actual && > > + test_cmp expect actual > > +' > > I'm not sure what we're really testing in the first one. The storage > unit conversion is the interesting bit. > > TBH, I'm not sure we need to separately test each possible type > conversion here. If we know that the type conversions are tested > elsewhere (and I would not be surprised if they're not, but let's assume > for a moment...) and we check that type conversions are applied to > "--default" values for some types, I don't think there's any reason to > assume that the others would not work. Agreed. I don't think that this test (and the ones that your comment below concerns) are testing anything meaningful, or that would catch any interesting future regressions. I have removed them from this series in v3. Thanks, Taylor
