| Message ID | 852ec003109b8244e2f9360ec64749779989c4a2.1631630356.git.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Upstreaming the Scalar command | expand |
On Tue, Sep 14 2021, Johannes Schindelin via GitGitGadget wrote: > +int cmd_main(int argc, const char **argv) > +{ > + struct strbuf scalar_usage = STRBUF_INIT; > + int i; > + > + if (argc > 1) { > + argv++; > + argc--; > + > + for (i = 0; builtins[i].name; i++) > + if (!strcmp(builtins[i].name, argv[0])) > + return !!builtins[i].fn(argc, argv); > + } > + > + strbuf_addstr(&scalar_usage, > + N_("scalar <command> [<options>]\n\nCommands:\n")); > + for (i = 0; builtins[i].name; i++) > + strbuf_addf(&scalar_usage, "\t%s\n", builtins[i].name); > + > + usage(scalar_usage.buf); > +} In 04/15 you continue and use the parse-options.c API, but here it's the usage.c API, which is generally being phased out. Any reason for the difference? It's preferrable not to add new usage() users if we can help it, I think we'll eventually want to remove it.
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > ... the > usage.c API, which is generally being phased out. That is news to me. Any reason why you think so?
On Fri, Sep 24 2021, Junio C Hamano wrote: > Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > >> ... the >> usage.c API, which is generally being phased out. > > That is news to me. Any reason why you think so? Perhaps better phrased as "generally going unused where we're using parse-options.c", although some quick historical trends I ran as an ad-hoc show usage() standing still since v1.6.0 in number of builtin*.c files[1], v.s. a 3x growth in usage_with_options() since then[2]. But in this case we're using an ad-hoc parser in cmd_main(), seemingly because it ends up copy/pasting a very small part of git.ci functionality over there, which is something that's been discussed as something we should move over to parse_options() sooner than later. It looks like a better approach to just use parse_options() consistently in scalar.c, as e.g. commit-graph.c, stash.c, multi-pack-index.c etc. that all implement a similar cmd/subcommand pattern do. The end-state of duplicating the "-C" and "-c" options from git.c can then easily be handled by parse-options.c, IIRC the stumbling point in migrating over git.c was some of the statefulness of other parts potentially needing incremenatl parsing (i.e. via parse_options_step() and friends). 1. $ parallel "printf "%s: " {} && git grep -l '\busage\(' {} -- 'builtin*.c' | wc -l" ::: v1.{1..9}.0 v2.{0..32}.0 v1.1.0:0 v1.2.0:0 v1.3.0:0 v1.4.0:20 v1.5.0:51 v1.7.0:35 v1.6.0:40 v1.8.0:34 v2.1.0:32 v1.9.0:33 v2.0.0:33 v2.2.0:32 v2.3.0:32 v2.4.0:32 v2.5.0:32 v2.6.0:31 v2.7.0:31 v2.9.0:29 v2.8.0:30 v2.11.0:29 v2.10.0:29 v2.12.0:29 v2.13.0:29 v2.14.0:31 v2.15.0:31 v2.16.0:31 v2.17.0:31 v2.19.0:32 v2.18.0:31 v2.22.0:31 v2.21.0:32 v2.24.0:31 v2.23.0:31 v2.20.0:32 v2.25.0:30 v2.26.0:30 v2.27.0:30 v2.32.0:29 v2.31.0:30 v2.29.0:31 v2.28.0:29 v2.30.0:31 2. $ parallel "printf "%s: " {} && git grep -l '\busage_with_options\(' {} -- 'builtin*.c' | wc -l" ::: v1.{1..9}.0 v2.{0..32}.0 v1.1.0:0 v1.2.0:0 v1.3.0:0 v1.4.0:0 v1.5.0:0 v1.6.0:19 v1.7.0:33 v1.8.0:42 v2.0.0:42 v1.9.0:42 v2.1.0:43 v2.2.0:43 v2.3.0:43 v2.4.0:43 v2.5.0:44 v2.6.0:45 v2.8.0:44 v2.10.0:45 v2.7.0:44 v2.9.0:45 v2.11.0:45 v2.12.0:45 v2.13.0:46 v2.14.0:47 v2.15.0:47 v2.16.0:47 v2.17.0:47 v2.19.0:50 v2.18.0:49 v2.20.0:52 v2.21.0:52 v2.22.0:53 v2.24.0:54 v2.23.0:54 v2.25.0:56 v2.26.0:55 v2.27.0:55 v2.28.0:55 v2.30.0:58 v2.32.0:60 v2.29.0:58 v2.31.0:58
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > On Fri, Sep 24 2021, Junio C Hamano wrote: > >> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: >> >>> ... the >>> usage.c API, which is generally being phased out. >> >> That is news to me. Any reason why you think so? > > Perhaps better phrased as "generally going unused where we're using > parse-options.c", ... Ah, if it was merely an observation of the general trend, then I agree. My reaction was primarily "Oh, why is somebody all of a sudden setting a project decision unilaterally, especially when even I do not do so very often myself? Did I miss recent discussion that resulted in an update to Documentation/ meant for developers?" Thanks.
diff --git a/Makefile b/Makefile index c3565fc0f8f..2d5c822f7a8 100644 --- a/Makefile +++ b/Makefile @@ -2447,6 +2447,10 @@ endif .PHONY: objects objects: $(OBJECTS) +SCALAR_SOURCES := contrib/scalar/scalar.c +SCALAR_OBJECTS := $(SCALAR_SOURCES:c=o) +OBJECTS += $(SCALAR_OBJECTS) + dep_files := $(foreach f,$(OBJECTS),$(dir $f).depend/$(notdir $f).d) dep_dirs := $(addsuffix .depend,$(sort $(dir $(OBJECTS)))) @@ -2586,6 +2590,10 @@ $(REMOTE_CURL_PRIMARY): remote-curl.o http.o http-walker.o GIT-LDFLAGS $(GITLIBS $(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \ $(CURL_LIBCURL) $(EXPAT_LIBEXPAT) $(LIBS) +contrib/scalar/scalar$X: $(SCALAR_OBJECTS) GIT-LDFLAGS $(GITLIBS) + $(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) \ + $(filter %.o,$^) $(LIBS) + $(LIB_FILE): $(LIB_OBJS) $(QUIET_AR)$(RM) $@ && $(AR) $(ARFLAGS) $@ $^ diff --git a/contrib/scalar/.gitignore b/contrib/scalar/.gitignore new file mode 100644 index 00000000000..ff3d47e84d0 --- /dev/null +++ b/contrib/scalar/.gitignore @@ -0,0 +1,2 @@ +/*.exe +/scalar diff --git a/contrib/scalar/Makefile b/contrib/scalar/Makefile new file mode 100644 index 00000000000..40c03ad10e1 --- /dev/null +++ b/contrib/scalar/Makefile @@ -0,0 +1,34 @@ +QUIET_SUBDIR0 = +$(MAKE) -C # space to separate -C and subdir +QUIET_SUBDIR1 = + +ifneq ($(findstring s,$(MAKEFLAGS)),s) +ifndef V + QUIET_SUBDIR0 = +@subdir= + QUIET_SUBDIR1 = ;$(NO_SUBDIR) echo ' ' SUBDIR $$subdir; \ + $(MAKE) $(PRINT_DIR) -C $$subdir +else + export V +endif +endif + +all: + +include ../../config.mak.uname +-include ../../config.mak.autogen +-include ../../config.mak + +TARGETS = scalar$(X) scalar.o +GITLIBS = ../../common-main.o ../../libgit.a ../../xdiff/lib.a + +all: scalar$X + +$(GITLIBS): + $(QUIET_SUBDIR0)../.. $(QUIET_SUBDIR1) $(subst ../../,,$@) + +$(TARGETS): $(GITLIBS) scalar.c + $(QUIET_SUBDIR0)../.. $(QUIET_SUBDIR1) $(patsubst %,contrib/scalar/%,$@) + +clean: + $(RM) $(TARGETS) + +.PHONY: all clean FORCE diff --git a/contrib/scalar/scalar.c b/contrib/scalar/scalar.c new file mode 100644 index 00000000000..7cff29e0fcd --- /dev/null +++ b/contrib/scalar/scalar.c @@ -0,0 +1,36 @@ +/* + * The Scalar command-line interface. + */ + +#include "cache.h" +#include "gettext.h" +#include "parse-options.h" + +static struct { + const char *name; + int (*fn)(int, const char **); +} builtins[] = { + { NULL, NULL}, +}; + +int cmd_main(int argc, const char **argv) +{ + struct strbuf scalar_usage = STRBUF_INIT; + int i; + + if (argc > 1) { + argv++; + argc--; + + for (i = 0; builtins[i].name; i++) + if (!strcmp(builtins[i].name, argv[0])) + return !!builtins[i].fn(argc, argv); + } + + strbuf_addstr(&scalar_usage, + N_("scalar <command> [<options>]\n\nCommands:\n")); + for (i = 0; builtins[i].name; i++) + strbuf_addf(&scalar_usage, "\t%s\n", builtins[i].name); + + usage(scalar_usage.buf); +}