Patrick Steinhardt <ps@xxxxxx> writes: > Hi, > > this is the fifth and hopefully last version of my patch sthat > introduces subcommands into git-config(1). > > The only changes compared to v4 are some fixes to commit messages. > Otherwise I'm not aware of any other feedback that would need to be > addressed. > > Patrick > > Patrick Steinhardt (14): > config: clarify memory ownership when preparing comment strings > builtin/config: move option array around > builtin/config: move "fixed-value" option to correct group > builtin/config: use `OPT_CMDMODE()` to specify modes > builtin/config: pull out function to handle config location > builtin/config: pull out function to handle `--null` > builtin/config: introduce "list" subcommand > builtin/config: introduce "get" subcommand > builtin/config: introduce "set" subcommand > builtin/config: introduce "unset" subcommand > builtin/config: introduce "rename-section" subcommand > builtin/config: introduce "remove-section" subcommand > builtin/config: introduce "edit" subcommand > builtin/config: display subcommand help > > Documentation/git-config.txt | 219 ++++++++------- > builtin/config.c | 512 ++++++++++++++++++++++++++++------- > config.c | 16 +- > config.h | 2 +- > t/t0450/txt-help-mismatches | 1 - > t/t1300-config.sh | 432 +++++++++++++++++------------ > 6 files changed, 812 insertions(+), 370 deletions(-) > > Range-diff against v4: > 1: 3aa26d5bff ! 1: 881d2b5426 config: clarify memory ownership when preparing comment strings > @@ Commit message > not like this micro-optimization really matters. Thus, callers are now > always responsible for freeing the value. > > + Signed-off-by: Patrick Steinhardt <ps@xxxxxx> > + > ## builtin/config.c ## > @@ builtin/config.c: static struct config_options config_options; > static int show_origin; > 2: 8f0804ab48 = 2: 66dffaa8f2 builtin/config: move option array around > 3: ddcd8031d7 ! 3: 36abda0e02 builtin/config: move "fixed-value" option to correct group > @@ Commit message > builtin/config: move "fixed-value" option to correct group > > The `--fixed-value` option can be used to alter how the value-pattern > - parameter is interpreted for the various submodes of git-config(1). But > - while it is an option, it is currently listed as part of the submodes > - group the command, which is wrong. > + parameter is interpreted for the various actions of git-config(1). But > + while it is an option, it is currently listed as part of the actions > + group, which is wrong. > > Move the option to the "Other" group, which hosts the various options > known to git-config(1). > 4: 1bc3918840 = 4: 34b66f9c87 builtin/config: use `OPT_CMDMODE()` to specify modes > 5: 3754812309 = 5: 4f90f206e7 builtin/config: pull out function to handle config location > 6: cb1714c493 = 6: df1a6f14e6 builtin/config: pull out function to handle `--null` > 7: b3f3c3ba6a ! 7: 1df76a9970 builtin/config: introduce "list" subcommand > @@ Commit message > builtin/config: introduce "list" subcommand > > While git-config(1) has several modes, those modes are not exposed with > - subcommands but instead by specifying e.g. `--unset` or `--list`. This > - 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`. > - Furthermore, to add to the confusion, git-config(1) also allows the user > - to request modes implicitly by just specifying the correct number of > - arguments. Thus, `git config foo.bar` will retrieve the value of > - "foo.bar" while `git config foo.bar baz` will set it to "baz". > + subcommands but instead by specifying action flags like `--unset` or > + `--list`. This 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`. Furthermore, to add to the confusion, git-config(1) also > + allows the user to request modes implicitly by just specifying the > + correct number of arguments. Thus, `git config foo.bar` will retrieve > + the value of "foo.bar" while `git config foo.bar baz` will set it to > + "baz". > > Overall, this makes for a confusing interface that could really use a > makeover. It hurts discoverability of what you can do with git-config(1) > 8: 0e6da909ac = 8: 29676b81e0 builtin/config: introduce "get" subcommand > 9: 8a623a31b9 = 9: 94afb5a5b7 builtin/config: introduce "set" subcommand > 10: e25e5b69cd = 10: e525c2326a builtin/config: introduce "unset" subcommand > 11: f24008d356 = 11: a797889890 builtin/config: introduce "rename-section" subcommand > 12: fc2ddd3201 = 12: 8ec214755e builtin/config: introduce "remove-section" subcommand > 13: 4c2d817eff = 13: 1893c23afc builtin/config: introduce "edit" subcommand > 14: 4c351b12b8 = 14: 97a48ab81d builtin/config: display subcommand help > > base-commit: d4cc1ec35f3bcce816b69986ca41943f6ce21377 > -- > 2.45.0 The range diff looks good. Overall I have nothing more to add to this series. Thanks Karthik
Attachment:
signature.asc
Description: PGP signature
