{"thread":{"id":"57314","subject":"[PATCH] scalar: accept -C and -c options before the subcommand","startedAt":"2022-01-26T11:15:36Z","lastAt":"2022-01-29T06:48:40Z","messageCount":14,"participants":["Johannes Schindelin via GitGitGadget","Taylor Blau","Ævar Arnfjörð Bjarmason","Derrick Stolee","Johannes Schindelin","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"446935","messageId":"pull.1130.git.1643195729608.gitgitgadget@gmail.com","threadId":"57314","inReplyTo":null,"subject":"[PATCH] scalar: accept -C and -c options before the subcommand","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-01-26T11:15:29Z","receivedAt":"2022-01-26T11:15:36Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nThe `git` executable has these two very useful options:\n\n-C :\n\tswitch to the specified directory before performing any actions\n\n-c =:\n\ttemporarily configure this setting for the duration of the\n\tspecified scalar subcommand\n\nWith this commit, we teach the `scalar` executable the same trick.\n\nSigned-off-by: Johannes Schindelin \n---\n scalar: accept -C and -c options\n \n This makes the scalar command a bit more handy by offering the same -c\n = and -C options as the git command.\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1130%2Fdscho%2Fscalar-c-and-C-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1130/dscho/scalar-c-and-C-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/1130\n\n contrib/scalar/scalar.c | 22 +++++++++++++++++++++-\n contrib/scalar/scalar.txt | 10 ++++++++++\n 2 files changed, 31 insertions(+), 1 deletion(-)\n\ndiff --git a/contrib/scalar/scalar.c b/contrib/scalar/scalar.c\nindex 1ce9c2b00e8..7db2a97416e 100644\n--- a/contrib/scalar/scalar.c\n+++ b/contrib/scalar/scalar.c\n@@ -808,6 +808,25 @@ int cmd_main(int argc, const char **argv)\n \tstruct strbuf scalar_usage = STRBUF_INIT;\n \tint i;\n \n+\twhile (argc > 1 && *argv[1] == '-') {\n+\t\tif (!strcmp(argv[1], \"-C\")) {\n+\t\t\tif (argc < 3)\n+\t\t\t\tdie(_(\"-C requires a \"));\n+\t\t\tif (chdir(argv[2]) < 0)\n+\t\t\t\tdie_errno(_(\"could not change to '%s'\"),\n+\t\t\t\t\t argv[2]);\n+\t\t\targc -= 2;\n+\t\t\targv += 2;\n+\t\t} else if (!strcmp(argv[1], \"-c\")) {\n+\t\t\tif (argc < 3)\n+\t\t\t\tdie(_(\"-c requires a = argument\"));\n+\t\t\tgit_config_push_parameter(argv[2]);\n+\t\t\targc -= 2;\n+\t\t\targv += 2;\n+\t\t} else\n+\t\t\tbreak;\n+\t}\n+\n \tif (argc > 1) {\n \t\targv++;\n \t\targc--;\n@@ -818,7 +837,8 @@ int cmd_main(int argc, const char **argv)\n \t}\n \n \tstrbuf_addstr(&scalar_usage,\n-\t\t N_(\"scalar []\\n\\nCommands:\\n\"));\n+\t\t N_(\"scalar [-C ] [-c =] \"\n+\t\t\t \" []\\n\\nCommands:\\n\"));\n \tfor (i = 0; builtins[i].name; i++)\n \t\tstrbuf_addf(&scalar_usage, \"\\t%s\\n\", builtins[i].name);\n \ndiff --git a/contrib/scalar/scalar.txt b/contrib/scalar/scalar.txt\nindex f416d637289..cf4e5b889cc 100644\n--- a/contrib/scalar/scalar.txt\n+++ b/contrib/scalar/scalar.txt\n@@ -36,6 +36,16 @@ The `scalar` command implements various subcommands, and different options\n depending on the subcommand. With the exception of `clone`, `list` and\n `reconfigure --all`, all subcommands expect to be run in an enlistment.\n \n+The following options can be specified _before_ the subcommand:\n+\n+-C ::\n+\tBefore running the subcommand, change the working directory. This\n+\toption imitates the same option of linkgit:git[1].\n+\n+-c =::\n+\tFor the duration of running the specified subcommand, configure this\n+\tsetting. This option imitates the same option of linkgit:git[1].\n+\n COMMANDS\n --------\n \n\nbase-commit: ddc35d833dd6f9e8946b09cecd3311b8aa18d295\n-- \ngitgitgadget\n"},{"id":"446996","messageId":"YfG0ybYOCwDzlbi5@nand.local","threadId":"57314","inReplyTo":"pull.1130.git.1643195729608.gitgitgadget@gmail.com","subject":"Re: [PATCH] scalar: accept -C and -c options before the subcommand","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2022-01-26T20:53:29Z","receivedAt":"2022-01-26T20:53:31Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Wed, Jan 26, 2022 at 11:15:29AM +0000, Johannes Schindelin via GitGitGadget wrote:\n> From: Johannes Schindelin \n>\n> The `git` executable has these two very useful options:\n>\n> -C :\n> \tswitch to the specified directory before performing any actions\n>\n> -c =:\n> \ttemporarily configure this setting for the duration of the\n> \tspecified scalar subcommand\n>\n> With this commit, we teach the `scalar` executable the same trick.\n>\n> Signed-off-by: Johannes Schindelin \n> ---\n> scalar: accept -C and -c options\n>\n> This makes the scalar command a bit more handy by offering the same -c\n> = and -C options as the git command.\n>\n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1130%2Fdscho%2Fscalar-c-and-C-v1\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1130/dscho/scalar-c-and-C-v1\n> Pull-Request: https://github.com/gitgitgadget/git/pull/1130\n>\n> contrib/scalar/scalar.c | 22 +++++++++++++++++++++-\n> contrib/scalar/scalar.txt | 10 ++++++++++\n> 2 files changed, 31 insertions(+), 1 deletion(-)\n>\n> diff --git a/contrib/scalar/scalar.c b/contrib/scalar/scalar.c\n> index 1ce9c2b00e8..7db2a97416e 100644\n> --- a/contrib/scalar/scalar.c\n> +++ b/contrib/scalar/scalar.c\n> @@ -808,6 +808,25 @@ int cmd_main(int argc, const char **argv)\n> \tstruct strbuf scalar_usage = STRBUF_INIT;\n> \tint i;\n>\n> +\twhile (argc > 1 && *argv[1] == '-') {\n> +\t\tif (!strcmp(argv[1], \"-C\")) {\n> +\t\t\tif (argc < 3)\n> +\t\t\t\tdie(_(\"-C requires a \"));\n> +\t\t\tif (chdir(argv[2]) < 0)\n> +\t\t\t\tdie_errno(_(\"could not change to '%s'\"),\n> +\t\t\t\t\t argv[2]);\n> +\t\t\targc -= 2;\n> +\t\t\targv += 2;\n> +\t\t} else if (!strcmp(argv[1], \"-c\")) {\n> +\t\t\tif (argc < 3)\n> +\t\t\t\tdie(_(\"-c requires a = argument\"));\n> +\t\t\tgit_config_push_parameter(argv[2]);\n> +\t\t\targc -= 2;\n> +\t\t\targv += 2;\n> +\t\t} else\n> +\t\t\tbreak;\n> +\t}\n> +\n\nAll looks right to me based on a cursory scan. It's too bad that we have\nto copy this code from git.c::handle_options().\n\nCould we call handle_options() (assuming that it was available to\nScalar's compilation unit) instead? I'm not sure if that's a naive\nquestion or not, but it might be nice to explain it out in the commit\nmessage in case other reviewers have the same question that I did.\n\nOn a more practical note: is there an easy way to test this? Like I\nsaid, I'm not necessarily worried about the code you wrote, just that it\nseems like this sort of thing *should* be easy enough to test. It's fine\nif there isn't, too, but again as somebody new to this area some\nexplanation would be helpful.\n\nThanks,\nTaylor\n"},{"id":"447032","messageId":"220127.86v8y5dgus.gmgdl@evledraar.gmail.com","threadId":"57314","inReplyTo":"pull.1130.git.1643195729608.gitgitgadget@gmail.com","subject":"Re: [PATCH] scalar: accept -C and -c options before the subcommand","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-01-27T02:55:58Z","receivedAt":"2022-01-27T02:58:58Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Wed, Jan 26 2022, Johannes Schindelin via GitGitGadget wrote:\n\n> From: Johannes Schindelin \n>\n> The `git` executable has these two very useful options:\n>\n> -C :\n> \tswitch to the specified directory before performing any actions\n>\n> -c =:\n> \ttemporarily configure this setting for the duration of the\n> \tspecified scalar subcommand\n>\n> With this commit, we teach the `scalar` executable the same trick.\n>\n> Signed-off-by: Johannes Schindelin \n> ---\n> scalar: accept -C and -c options\n> \n> This makes the scalar command a bit more handy by offering the same -c\n> = and -C options as the git command.\n>\n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1130%2Fdscho%2Fscalar-c-and-C-v1\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1130/dscho/scalar-c-and-C-v1\n> Pull-Request: https://github.com/gitgitgadget/git/pull/1130\n\nI think it would help for context to note that this patch had at least 6\nsubmissions on the ML already as part of early versions of the scalar\nseries.\n\nHere's the CL of the iteration that ejected it:\nhttps://lore.kernel.org/git/pull.1005.v7.git.1637158762.gitgitgadget@gmail.com/\n\nWhere you summarized:\n \n * The patch that adds support for -c = and -C was\n moved to its own add-on patch series: While it is obvious that those\n options are valuable to have, an open question is whether there are other\n \"pre-command\" options in git that would be useful, too, and I would like\n to postpone that discussion to that date.\n\nHaving been involved in those discussions I can't remember what the\npre-command options you're referring to there are, but it seems \"that\ndate\" is probably upon us.\n"},{"id":"447081","messageId":"0f8d5d04-e86c-48e2-fea0-32c25c3f9325@gmail.com","threadId":"57314","inReplyTo":"220127.86v8y5dgus.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH] scalar: accept -C and -c options before the subcommand","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2022-01-27T14:46:55Z","receivedAt":"2022-01-27T14:46:59Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 1/26/2022 9:55 PM, Ævar Arnfjörð Bjarmason wrote:\n> \n> On Wed, Jan 26 2022, Johannes Schindelin via GitGitGadget wrote:\n> \n>> From: Johannes Schindelin \n>>\n>> The `git` executable has these two very useful options:\n>>\n>> -C :\n>> \tswitch to the specified directory before performing any actions\n>>\n>> -c =:\n>> \ttemporarily configure this setting for the duration of the\n>> \tspecified scalar subcommand\n>>\n>> With this commit, we teach the `scalar` executable the same trick.\n>>\n>> Signed-off-by: Johannes Schindelin \n>> ---\n>> scalar: accept -C and -c options\n>> \n>> This makes the scalar command a bit more handy by offering the same -c\n>> = and -C options as the git command.\n>>\n>> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1130%2Fdscho%2Fscalar-c-and-C-v1\n>> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1130/dscho/scalar-c-and-C-v1\n>> Pull-Request: https://github.com/gitgitgadget/git/pull/1130\n> \n> I think it would help for context to note that this patch had at least 6\n> submissions on the ML already as part of early versions of the scalar\n> series.\n> \n> Here's the CL of the iteration that ejected it:\n> https://lore.kernel.org/git/pull.1005.v7.git.1637158762.gitgitgadget@gmail.com/\n> \n> Where you summarized:\n> \n> * The patch that adds support for -c = and -C was\n> moved to its own add-on patch series: While it is obvious that those\n> options are valuable to have, an open question is whether there are other\n> \"pre-command\" options in git that would be useful, too, and I would like\n> to postpone that discussion to that date.\n> \n> Having been involved in those discussions I can't remember what the\n> pre-command options you're referring to there are, but it seems \"that\n> date\" is probably upon us.\n\nMy understanding was that this was ejected so we could find the right time\nto lib-ify handle_options() (as Taylor suggested), but we didn't want to do\nthat while Scalar was still in a tentative state (in contrib/ with a plan\nto move it out after more is implemented).\n\nThe biggest benefits of using handle_options() is for other pre-command\noptions such as --exec-path, which I use on a regular basis when testing\nnew functionality.\n\nThere are other options in handle_options() that might not be appropriate,\nor might be incorrect if we just make handle_options() non-static. For\nexample, `scalar --list-cmds=parseopt` wouldn't show the scalar commands\nand would instead show the git commands.\n\nSo my feeling is that we should continue to delay this functionality until\nScalar is more stable, perhaps even until after it moves out of contrib/.\nThe need to change handle_options() to work with a new top-level command\nis novel enough to be worth careful scrutiny, but that effort is only\nvaluable if the Git community is more committed to having Scalar in the\ntree for the long term.\n\nThanks,\n-Stolee\n"},{"id":"447191","messageId":"nycvar.QRO.7.76.6.2201281148310.347@tvgsbejvaqbjf.bet","threadId":"57314","inReplyTo":"0f8d5d04-e86c-48e2-fea0-32c25c3f9325@gmail.com","subject":"Re: [PATCH] scalar: accept -C and -c options before the subcommand","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2022-01-28T11:27:19Z","receivedAt":"2022-01-28T11:27:28Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Stolee,\n\nOn Thu, 27 Jan 2022, Derrick Stolee wrote:\n\n> The biggest benefits of using handle_options() is for other pre-command\n> options such as --exec-path, which I use on a regular basis when testing\n> new functionality.\n>\n> There are other options in handle_options() that might not be\n> appropriate, or might be incorrect if we just make handle_options()\n> non-static. For example, `scalar --list-cmds=parseopt` wouldn't show the\n> scalar commands and would instead show the git commands.\n\nRight, and since `handle_options()` lives in the same file as `git`'s\n`cmd_main()` function, we would not only have to disentangle options that\nwork only for `git` from those that would also work for `scalar`, but we\nwould have to extract the `handle_options()` function into a separate\nfile.\n\nAnd while at it, a tangent someone with infinite time on their hands might\nsuggest is: why not convert `handle_options()` to the `parse_options()`\nmachinery? Which would of course solve one issue by adding several new\nones. Don't get me wrong: I would find it useful to convert\n`git.c:handle_options()` to a function in `libgit.a` which uses the\n`parse_options()` machinery. It'll just require a lot of time, and I do\nnot see enough benefit that would make it worth embarking on that\nparticular journey.\n\nBut since I had a look at `handle_options()` anyway, I might just as well\nsummarize my insights about how applicable the supported options are for\n`scalar` here:\n\n# Beneficial\n\n -c =\n --config-env =\n -C \n\n\tSince I added support for these (except for the long form\n\t`--config-env` that I actually only learned while researching this\n\temail), it is obvious that I'd like `scalar` to support them.\n\n# Won't hurt\n\n --html-path\n --man-path\n --info-path\n\n\tSure, for `scalar help` (which I implement in a patch series I\n\thave not yet formally contributed to the Git mailing list), these\n\toptions might even make some sense.\n\n --paginate\n --no-pager\n\n\tThere are no Scalar commands that would benefit from using the\n\tpager. But if we parse those options, we also have to handle them.\n\tWhich would mean extracting _even more_ code from `git.c` just so\n\tthat we can reuse `handle_options()` in Scalar.\n\n --no-replace-objects\n --git-dir\n --work-tree\n\n\tThese options would only be relevant to the `scalar run` command,\n\tno other Scalar command works on an existing Git worktree. And\n\teven for that command, I doubt that they are actually useful in\n\tScalar's context.\n\n --namespace\n\n\tThis option seems relevant mostly for repositories that are served\n\tup for cloning and fetching, which is not what the Scalar command\n\tsupports directly.\n\n --super-prefix\n --bare\n\n\tThese options do not make sense in Scalar's context (it's neither\n\tabout bare repositories not about submodules). That is the case\n\tfor most `git` commands, too, of course.\n\n --literal-pathspecs\n --no-literal-pathspecs\n --glob-pathspecs\n --noglob-pathspecs\n --icase-pathspecs\n --no-optional-locks\n\n\tNone of Scalar's commands take pathspecs, so these options won't\n\thave any effect.\n\n --shallow-file\n\n\tThis option (which I learned about today) is purposefully not\n\tdocumented, as it is merely an internal option for use e.g. by\n\t`receive-pack`.\n\n# Detrimental\n\n --exec-path\n\n\tSince `scalar` is tightly coupled to a specific Git version, it\n\twould cause much more harm than benefit to encourage users to use\n\ta different Git version by offering them this option.\n\n --list-cmds\n\n\tAs you pointed out, this option would produce misleading output.\n\nGiven that only the `-c` and `-C` options are _actually_ useful in the\ncontext of the `scalar` command, I would argue that I chose the best\napproach, as the benefit of the intrusive refactorings that would be\nnecessary to share code with `git.c` is rather small compared with the\namount of work.\n\n> So my feeling is that we should continue to delay this functionality\n> until Scalar is more stable, perhaps even until after it moves out of\n> contrib/. The need to change handle_options() to work with a new\n> top-level command is novel enough to be worth careful scrutiny, but that\n> effort is only valuable if the Git community is more committed to having\n> Scalar in the tree for the long term.\n\nI am okay with holding off with this, for now.\n\nOn the other hand, as I pointed out above: I do not really see it worth\nthe effort to refactor `git.c:handle_options()` for the minimal benefit it\nwould give us over the approach I chose in the patch under discussion.\n\nCiao,\nDscho\n"},{"id":"447192","messageId":"nycvar.QRO.7.76.6.2201281227330.347@tvgsbejvaqbjf.bet","threadId":"57314","inReplyTo":"YfG0ybYOCwDzlbi5@nand.local","subject":"Re: [PATCH] scalar: accept -C and -c options before the subcommand","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2022-01-28T11:43:44Z","receivedAt":"2022-01-28T11:43:51Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Taylor,\n\nOn Wed, 26 Jan 2022, Taylor Blau wrote:\n\n> On Wed, Jan 26, 2022 at 11:15:29AM +0000, Johannes Schindelin via GitGitGadget wrote:\n> > From: Johannes Schindelin \n> >\n> > The `git` executable has these two very useful options:\n> >\n> > -C :\n> > \tswitch to the specified directory before performing any actions\n> >\n> > -c =:\n> > \ttemporarily configure this setting for the duration of the\n> > \tspecified scalar subcommand\n> >\n> > With this commit, we teach the `scalar` executable the same trick.\n> >\n> > Signed-off-by: Johannes Schindelin \n> > ---\n> > scalar: accept -C and -c options\n> >\n> > This makes the scalar command a bit more handy by offering the same -c\n> > = and -C options as the git command.\n> >\n> > Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1130%2Fdscho%2Fscalar-c-and-C-v1\n> > Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1130/dscho/scalar-c-and-C-v1\n> > Pull-Request: https://github.com/gitgitgadget/git/pull/1130\n> >\n> > contrib/scalar/scalar.c | 22 +++++++++++++++++++++-\n> > contrib/scalar/scalar.txt | 10 ++++++++++\n> > 2 files changed, 31 insertions(+), 1 deletion(-)\n> >\n> > diff --git a/contrib/scalar/scalar.c b/contrib/scalar/scalar.c\n> > index 1ce9c2b00e8..7db2a97416e 100644\n> > --- a/contrib/scalar/scalar.c\n> > +++ b/contrib/scalar/scalar.c\n> > @@ -808,6 +808,25 @@ int cmd_main(int argc, const char **argv)\n> > \tstruct strbuf scalar_usage = STRBUF_INIT;\n> > \tint i;\n> >\n> > +\twhile (argc > 1 && *argv[1] == '-') {\n> > +\t\tif (!strcmp(argv[1], \"-C\")) {\n> > +\t\t\tif (argc < 3)\n> > +\t\t\t\tdie(_(\"-C requires a \"));\n> > +\t\t\tif (chdir(argv[2]) < 0)\n> > +\t\t\t\tdie_errno(_(\"could not change to '%s'\"),\n> > +\t\t\t\t\t argv[2]);\n> > +\t\t\targc -= 2;\n> > +\t\t\targv += 2;\n> > +\t\t} else if (!strcmp(argv[1], \"-c\")) {\n> > +\t\t\tif (argc < 3)\n> > +\t\t\t\tdie(_(\"-c requires a = argument\"));\n> > +\t\t\tgit_config_push_parameter(argv[2]);\n> > +\t\t\targc -= 2;\n> > +\t\t\targv += 2;\n> > +\t\t} else\n> > +\t\t\tbreak;\n> > +\t}\n> > +\n>\n> All looks right to me based on a cursory scan. It's too bad that we have\n> to copy this code from git.c::handle_options().\n\nIt's only a dozen lines, though, and they are pretty stable, so I doubt\nthat we risk divergent copied code.\n\n> Could we call handle_options() (assuming that it was available to\n> Scalar's compilation unit) instead? I'm not sure if that's a naive\n> question or not, but it might be nice to explain it out in the commit\n> message in case other reviewers have the same question that I did.\n\nI just responded to Stolee elsewhere in this thread with a lengthy\nanalysis of the options, and the conclusion that it would not be worth the\neffort to refactor `handle_options()`.\n\n> On a more practical note: is there an easy way to test this?\n\nIt would be pretty easy to test `-C`:\n\n\tgit init sub &&\n\tscalar -C sub register &&\n\t[... verify that `sub/` is now a Scalar repository ...]\n\nFor `-c`, we would need to configure something parsed by\n`git_default_config()` that would influence what `scalar register` does,\nthen verify that. Or even better, use a config setting that is in the\n\"Optional\" section of `set_recommended_config()`, i.e. it will refuse to\noverride an already-configured value. Something like `status.aheadBehind`.\n\nI added this:\n\n-- snip --\ndiff --git a/contrib/scalar/t/t9099-scalar.sh b/contrib/scalar/t/t9099-scalar.sh\nindex 2e1502ad45e..89781568f43 100755\n--- a/contrib/scalar/t/t9099-scalar.sh\n+++ b/contrib/scalar/t/t9099-scalar.sh\n@@ -85,4 +85,12 @@ test_expect_success 'scalar delete with enlistment' '\n \ttest_path_is_missing cloned\n '\n\n+test_expect_success 'scalar supports -c/-C' '\n+\ttest_when_finished \"scalar delete sub\" &&\n+\tgit init sub &&\n+\tscalar -C sub -c status.aheadBehind=bogus register &&\n+\ttest -z \"$(git -C sub config --local status.aheadBehind)\" &&\n+\ttest true = \"$(git -C sub config core.preloadIndex)\"\n+'\n+\n test_done\n-- snap --\n\nCiao,\nDscho\n"},{"id":"447215","messageId":"pull.1130.v2.git.1643380317358.gitgitgadget@gmail.com","threadId":"57314","inReplyTo":"pull.1130.git.1643195729608.gitgitgadget@gmail.com","subject":"[PATCH v2] scalar: accept -C and -c options before the subcommand","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-01-28T14:31:57Z","receivedAt":"2022-01-28T14:32:04Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nThe `git` executable has these two very useful options:\n\n-C :\n\tswitch to the specified directory before performing any actions\n\n-c =:\n\ttemporarily configure this setting for the duration of the\n\tspecified scalar subcommand\n\nWith this commit, we teach the `scalar` executable the same trick.\n\nNote: It might look like a good idea to try to reuse the\n`handle_options()` function in `git.c` instead of replicating only the\n`-c`/`-C` part. However, that function is not only not in `libgit.a`, it\nis also intricately entangled with the rest of the code in `git.c` that\nis necessary e.g. to handle `--paginate`. Besides, no other option\nhandled by that `handle_options()` function is relevant to Scalar,\ntherefore the cost of refactoring vastly would outweigh the benefit.\n\nSigned-off-by: Johannes Schindelin \n---\n scalar: accept -C and -c options\n \n This makes the scalar command a bit more handy by offering the same -c\n = and -C options as the git command.\n \n Changes since v1:\n \n * Added a regression test case.\n * Augmented the commit message with a brief analysis why we're choosing\n not to refactor git.c:handle_options() but instead copy-edit the\n dozen or so lines that we want.\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1130%2Fdscho%2Fscalar-c-and-C-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1130/dscho/scalar-c-and-C-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/1130\n\nRange-diff vs v1:\n\n 1: 8f2af8c3ec1 ! 1: d7ee2d03b04 scalar: accept -C and -c options before the subcommand\n @@ Commit message\n \n With this commit, we teach the `scalar` executable the same trick.\n \n + Note: It might look like a good idea to try to reuse the\n + `handle_options()` function in `git.c` instead of replicating only the\n + `-c`/`-C` part. However, that function is not only not in `libgit.a`, it\n + is also intricately entangled with the rest of the code in `git.c` that\n + is necessary e.g. to handle `--paginate`. Besides, no other option\n + handled by that `handle_options()` function is relevant to Scalar,\n + therefore the cost of refactoring vastly would outweigh the benefit.\n +\n Signed-off-by: Johannes Schindelin \n \n ## contrib/scalar/scalar.c ##\n @@ contrib/scalar/scalar.txt: The `scalar` command implements various subcommands,\n COMMANDS\n --------\n \n +\n + ## contrib/scalar/t/t9099-scalar.sh ##\n +@@ contrib/scalar/t/t9099-scalar.sh: test_expect_success 'scalar delete with enlistment' '\n + \ttest_path_is_missing cloned\n + '\n + \n ++test_expect_success 'scalar supports -c/-C' '\n ++\ttest_when_finished \"scalar delete sub\" &&\n ++\tgit init sub &&\n ++\tscalar -C sub -c status.aheadBehind=bogus register &&\n ++\ttest -z \"$(git -C sub config --local status.aheadBehind)\" &&\n ++\ttest true = \"$(git -C sub config core.preloadIndex)\"\n ++'\n ++\n + test_done\n\n\n contrib/scalar/scalar.c | 22 +++++++++++++++++++++-\n contrib/scalar/scalar.txt | 10 ++++++++++\n contrib/scalar/t/t9099-scalar.sh | 8 ++++++++\n 3 files changed, 39 insertions(+), 1 deletion(-)\n\ndiff --git a/contrib/scalar/scalar.c b/contrib/scalar/scalar.c\nindex 1ce9c2b00e8..7db2a97416e 100644\n--- a/contrib/scalar/scalar.c\n+++ b/contrib/scalar/scalar.c\n@@ -808,6 +808,25 @@ int cmd_main(int argc, const char **argv)\n \tstruct strbuf scalar_usage = STRBUF_INIT;\n \tint i;\n \n+\twhile (argc > 1 && *argv[1] == '-') {\n+\t\tif (!strcmp(argv[1], \"-C\")) {\n+\t\t\tif (argc < 3)\n+\t\t\t\tdie(_(\"-C requires a \"));\n+\t\t\tif (chdir(argv[2]) < 0)\n+\t\t\t\tdie_errno(_(\"could not change to '%s'\"),\n+\t\t\t\t\t argv[2]);\n+\t\t\targc -= 2;\n+\t\t\targv += 2;\n+\t\t} else if (!strcmp(argv[1], \"-c\")) {\n+\t\t\tif (argc < 3)\n+\t\t\t\tdie(_(\"-c requires a = argument\"));\n+\t\t\tgit_config_push_parameter(argv[2]);\n+\t\t\targc -= 2;\n+\t\t\targv += 2;\n+\t\t} else\n+\t\t\tbreak;\n+\t}\n+\n \tif (argc > 1) {\n \t\targv++;\n \t\targc--;\n@@ -818,7 +837,8 @@ int cmd_main(int argc, const char **argv)\n \t}\n \n \tstrbuf_addstr(&scalar_usage,\n-\t\t N_(\"scalar []\\n\\nCommands:\\n\"));\n+\t\t N_(\"scalar [-C ] [-c =] \"\n+\t\t\t \" []\\n\\nCommands:\\n\"));\n \tfor (i = 0; builtins[i].name; i++)\n \t\tstrbuf_addf(&scalar_usage, \"\\t%s\\n\", builtins[i].name);\n \ndiff --git a/contrib/scalar/scalar.txt b/contrib/scalar/scalar.txt\nindex f416d637289..cf4e5b889cc 100644\n--- a/contrib/scalar/scalar.txt\n+++ b/contrib/scalar/scalar.txt\n@@ -36,6 +36,16 @@ The `scalar` command implements various subcommands, and different options\n depending on the subcommand. With the exception of `clone`, `list` and\n `reconfigure --all`, all subcommands expect to be run in an enlistment.\n \n+The following options can be specified _before_ the subcommand:\n+\n+-C ::\n+\tBefore running the subcommand, change the working directory. This\n+\toption imitates the same option of linkgit:git[1].\n+\n+-c =::\n+\tFor the duration of running the specified subcommand, configure this\n+\tsetting. This option imitates the same option of linkgit:git[1].\n+\n COMMANDS\n --------\n \ndiff --git a/contrib/scalar/t/t9099-scalar.sh b/contrib/scalar/t/t9099-scalar.sh\nindex 2e1502ad45e..89781568f43 100755\n--- a/contrib/scalar/t/t9099-scalar.sh\n+++ b/contrib/scalar/t/t9099-scalar.sh\n@@ -85,4 +85,12 @@ test_expect_success 'scalar delete with enlistment' '\n \ttest_path_is_missing cloned\n '\n \n+test_expect_success 'scalar supports -c/-C' '\n+\ttest_when_finished \"scalar delete sub\" &&\n+\tgit init sub &&\n+\tscalar -C sub -c status.aheadBehind=bogus register &&\n+\ttest -z \"$(git -C sub config --local status.aheadBehind)\" &&\n+\ttest true = \"$(git -C sub config core.preloadIndex)\"\n+'\n+\n test_done\n\nbase-commit: ddc35d833dd6f9e8946b09cecd3311b8aa18d295\n-- \ngitgitgadget\n"},{"id":"447228","messageId":"xmqqv8y3wxas.fsf@gitster.g","threadId":"57314","inReplyTo":"0f8d5d04-e86c-48e2-fea0-32c25c3f9325@gmail.com","subject":"Re: [PATCH] scalar: accept -C and -c options before the subcommand","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-01-28T18:05:31Z","receivedAt":"2022-01-28T18:05:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Derrick Stolee writes:\n\n> My understanding was that this was ejected so we could find the right time\n> to lib-ify handle_options() (as Taylor suggested), but we didn't want to do\n> that while Scalar was still in a tentative state (in contrib/ with a plan\n> to move it out after more is implemented).\n>\n> The biggest benefits of using handle_options() is for other pre-command\n> options such as --exec-path, which I use on a regular basis when testing\n> new functionality.\n>\n> There are other options in handle_options() that might not be appropriate,\n> or might be incorrect if we just make handle_options() non-static. For\n> example, `scalar --list-cmds=parseopt` wouldn't show the scalar commands\n> and would instead show the git commands.\n>\n> So my feeling is that we should continue to delay this functionality until\n> Scalar is more stable, perhaps even until after it moves out of contrib/.\n> The need to change handle_options() to work with a new top-level command\n> is novel enough to be worth careful scrutiny, but that effort is only\n> valuable if the Git community is more committed to having Scalar in the\n> tree for the long term.\n\nThe usual caveat that little things tend to accumulate and you can\ndie from thousands of paper cuts aside, if \"run in that directory\"\nand \"pretend these configuration variables are set\" are useful\nenough features to Scalar, I think that the straight-forward option\nparser this patch adds is very much acceptable.\n\nIf we need more options (i.e. when we need to add the third thing),\nthat would be the best time to see how handle_options() can be\nrestructured to serve the different set of options git and Scaler\nneed.\n\nAnd this loop, which is too small to be even called \"duplicated\nimplementation\", should suffice in the meantime.\n\n\n"},{"id":"447231","messageId":"220128.8635l7d7y6.gmgdl@evledraar.gmail.com","threadId":"57314","inReplyTo":"nycvar.QRO.7.76.6.2201281148310.347@tvgsbejvaqbjf.bet","subject":"Re: [PATCH] scalar: accept -C and -c options before the subcommand","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-01-28T18:21:38Z","receivedAt":"2022-01-28T18:35:51Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Fri, Jan 28 2022, Johannes Schindelin wrote:\n\n> On Thu, 27 Jan 2022, Derrick Stolee wrote:\n>\n>> The biggest benefits of using handle_options() is for other pre-command\n>> options such as --exec-path, which I use on a regular basis when testing\n>> new functionality.\n>>\n>> There are other options in handle_options() that might not be\n>> appropriate, or might be incorrect if we just make handle_options()\n>> non-static. For example, `scalar --list-cmds=parseopt` wouldn't show the\n>> scalar commands and would instead show the git commands.\n>\n> Right, and since `handle_options()` lives in the same file as `git`'s\n> `cmd_main()` function, we would not only have to disentangle options that\n> work only for `git` from those that would also work for `scalar`, but we\n> would have to extract the `handle_options()` function into a separate\n> file.\n>\n> And while at it, a tangent someone with infinite time on their hands might\n> suggest is: why not convert `handle_options()` to the `parse_options()`\n> machinery? Which would of course solve one issue by adding several new\n> ones. Don't get me wrong: I would find it useful to convert\n> `git.c:handle_options()` to a function in `libgit.a` which uses the\n> `parse_options()` machinery. It'll just require a lot of time, and I do\n> not see enough benefit that would make it worth embarking on that\n> particular journey.\n>\n> But since I had a look at `handle_options()` anyway, I might just as well\n> summarize my insights about how applicable the supported options are for\n> `scalar` here:\n> [...]\n> # Detrimental\n>\n> --exec-path\n>\n> \tSince `scalar` is tightly coupled to a specific Git version, it\n> \twould cause much more harm than benefit to encourage users to use\n> \ta different Git version by offering them this option.\n\nSo just to clarify, do you and Stolee disagree about scalar supporting\n--exec-path, per his comments above?\n\n>\n> --list-cmds\n>\n> \tAs you pointed out, this option would produce misleading output.\n>\n> Given that only the `-c` and `-C` options are _actually_ useful in the\n> context of the `scalar` command, I would argue that I chose the best\n> approach, as the benefit of the intrusive refactorings that would be\n> necessary to share code with `git.c` is rather small compared with the\n> amount of work.\n>\n>> So my feeling is that we should continue to delay this functionality\n>> until Scalar is more stable, perhaps even until after it moves out of\n>> contrib/. The need to change handle_options() to work with a new\n>> top-level command is novel enough to be worth careful scrutiny, but that\n>> effort is only valuable if the Git community is more committed to having\n>> Scalar in the tree for the long term.\n>\n> I am okay with holding off with this, for now.\n>\n> On the other hand, as I pointed out above: I do not really see it worth\n> the effort to refactor `git.c:handle_options()` for the minimal benefit it\n> would give us over the approach I chose in the patch under discussion.\n\nI'm fine with just integrating this patch as-is, I just wanted to chime\nin upthread with a question/context about a thing stated in previous\ndiscussion/a link to that discussion.\n\nIt does seem to me that you're making a mountain out of a molehill\nhere. The below quick hack is a lib-ification of -c, -C and --config-env\nthat took me around 10 minutes. For ease of reading I squashed it into\nyour patch.\n\nI mean, we're just talking about copy/pasting existing working code to a\nnew header file, giving it a function parameter, and other small bits of\nscaffolding.\n\nOne thing we'll be doing by doing that is not wasting the time of\n(checks $(ls -l po/*.po|wc -l) ...) around 20 translator on 2 new\nstrings you introduced, and if you view it with the move detection you\ncan see it's almost entirely just moving existing code around.\n\nIn this case I don't mind much, but speaking generally I see you and\nStolee tying yourselves in knots again about scalar being in contrib so\nwe shouldn't use libgit.\n\nIt already uses libgit, there's even (last I checked) at least one\nfunction in it only used directly by the scalar code.\n\nI don't remember anyone having any objection to scalar using libgit\ncode, or even that there's libgit code just to help it along. That's a\nself-imposed limitation you two seem to have invented.\n\nPersonally I find a patch like the below much easier to review. It's the\nparts that aren't easy to review boilerplate are all things that we have\nin-tree already.\n\nWhereas proposing a new way to parse -c or -C will lead (at least me) to\ncarefully eyeballing that new implementation, looking at how it differs\n(if at all) from the existing one, wondering why the i18n strings are\nsubtly different etc (I saw one reason is that since the code was\ncopy/pasted initially the git.c version was updated, but your patch\nwasn't updated to copy it).\n\ndiff --git a/Makefile b/Makefile\nindex 5580859afdb..2d0a6611cd5 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -900,6 +900,7 @@ LIB_OBJS += fmt-merge-msg.o\n LIB_OBJS += fsck.o\n LIB_OBJS += fsmonitor.o\n LIB_OBJS += gettext.o\n+LIB_OBJS += gitcmd.o\n LIB_OBJS += gpg-interface.o\n LIB_OBJS += graph.o\n LIB_OBJS += grep.o\ndiff --git a/contrib/scalar/scalar.c b/contrib/scalar/scalar.c\nindex 1ce9c2b00e8..ee793ff6ccc 100644\n--- a/contrib/scalar/scalar.c\n+++ b/contrib/scalar/scalar.c\n@@ -11,6 +11,7 @@\n #include \"dir.h\"\n #include \"packfile.h\"\n #include \"help.h\"\n+#include \"gitcmd.h\"\n \n /*\n * Remove the deepest subdirectory in the provided path string. Path must not\n@@ -808,17 +809,33 @@ int cmd_main(int argc, const char **argv)\n \tstruct strbuf scalar_usage = STRBUF_INIT;\n \tint i;\n \n-\tif (argc > 1) {\n \targv++;\n \targc--;\n \n+\twhile (argc > 1 && *argv[0] == '-') {\n+\t\tint show_usage = 0;\n+\n+\t\tif (gity_handle_options(&argv, &argc, NULL, &show_usage)) {\n+\t\t\t(argv)++;\n+\t\t\t(argc)--;\n+\n+\t\t\tif (show_usage)\n+\t\t\t\tgoto usage;\n+\t\t} else {\n+\t\t\tbreak;\n+\t\t}\n+\t}\n+\n+\tif (argc) {\n \t\tfor (i = 0; builtins[i].name; i++)\n \t\t\tif (!strcmp(builtins[i].name, argv[0]))\n \t\t\t\treturn !!builtins[i].fn(argc, argv);\n \t}\n \n+usage:\n \tstrbuf_addstr(&scalar_usage,\n-\t\t N_(\"scalar []\\n\\nCommands:\\n\"));\n+\t\t N_(\"scalar [-C ] [-c =] \"\n+\t\t\t \" []\\n\\nCommands:\\n\"));\n \tfor (i = 0; builtins[i].name; i++)\n \t\tstrbuf_addf(&scalar_usage, \"\\t%s\\n\", builtins[i].name);\n\ndiff --git a/Makefile b/Makefile\nindex 5580859afdb..2d0a6611cd5 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -900,6 +900,7 @@ LIB_OBJS += fmt-merge-msg.o\n LIB_OBJS += fsck.o\n LIB_OBJS += fsmonitor.o\n LIB_OBJS += gettext.o\n+LIB_OBJS += gitcmd.o\n LIB_OBJS += gpg-interface.o\n LIB_OBJS += graph.o\n LIB_OBJS += grep.o\ndiff --git a/contrib/scalar/scalar.c b/contrib/scalar/scalar.c\nindex 1ce9c2b00e8..ee793ff6ccc 100644\n--- a/contrib/scalar/scalar.c\n+++ b/contrib/scalar/scalar.c\n@@ -11,6 +11,7 @@\n #include \"dir.h\"\n #include \"packfile.h\"\n #include \"help.h\"\n+#include \"gitcmd.h\"\n \n /*\n * Remove the deepest subdirectory in the provided path string. Path must not\n@@ -808,17 +809,33 @@ int cmd_main(int argc, const char **argv)\n \tstruct strbuf scalar_usage = STRBUF_INIT;\n \tint i;\n \n-\tif (argc > 1) {\n \targv++;\n \targc--;\n \n+\twhile (argc > 1 && *argv[0] == '-') {\n+\t\tint show_usage = 0;\n+\n+\t\tif (gity_handle_options(&argv, &argc, NULL, &show_usage)) {\n+\t\t\t(argv)++;\n+\t\t\t(argc)--;\n+\n+\t\t\tif (show_usage)\n+\t\t\t\tgoto usage;\n+\t\t} else {\n+\t\t\tbreak;\n+\t\t}\n+\t}\n+\n+\tif (argc) {\n \t\tfor (i = 0; builtins[i].name; i++)\n \t\t\tif (!strcmp(builtins[i].name, argv[0]))\n \t\t\t\treturn !!builtins[i].fn(argc, argv);\n \t}\n \n+usage:\n \tstrbuf_addstr(&scalar_usage,\n-\t\t N_(\"scalar []\\n\\nCommands:\\n\"));\n+\t\t N_(\"scalar [-C ] [-c =] \"\n+\t\t\t \" []\\n\\nCommands:\\n\"));\n \tfor (i = 0; builtins[i].name; i++)\n \t\tstrbuf_addf(&scalar_usage, \"\\t%s\\n\", builtins[i].name);\n \ndiff --git a/contrib/scalar/scalar.txt b/contrib/scalar/scalar.txt\nindex f416d637289..cf4e5b889cc 100644\n--- a/contrib/scalar/scalar.txt\n+++ b/contrib/scalar/scalar.txt\n@@ -36,6 +36,16 @@ The `scalar` command implements various subcommands, and different options\n depending on the subcommand. With the exception of `clone`, `list` and\n `reconfigure --all`, all subcommands expect to be run in an enlistment.\n \n+The following options can be specified _before_ the subcommand:\n+\n+-C ::\n+\tBefore running the subcommand, change the working directory. This\n+\toption imitates the same option of linkgit:git[1].\n+\n+-c =::\n+\tFor the duration of running the specified subcommand, configure this\n+\tsetting. This option imitates the same option of linkgit:git[1].\n+\n COMMANDS\n --------\n \ndiff --git a/contrib/scalar/t/t9099-scalar.sh b/contrib/scalar/t/t9099-scalar.sh\nindex 2e1502ad45e..89781568f43 100755\n--- a/contrib/scalar/t/t9099-scalar.sh\n+++ b/contrib/scalar/t/t9099-scalar.sh\n@@ -85,4 +85,12 @@ test_expect_success 'scalar delete with enlistment' '\n \ttest_path_is_missing cloned\n '\n \n+test_expect_success 'scalar supports -c/-C' '\n+\ttest_when_finished \"scalar delete sub\" &&\n+\tgit init sub &&\n+\tscalar -C sub -c status.aheadBehind=bogus register &&\n+\ttest -z \"$(git -C sub config --local status.aheadBehind)\" &&\n+\ttest true = \"$(git -C sub config core.preloadIndex)\"\n+'\n+\n test_done\ndiff --git a/git.c b/git.c\nindex edda922ce6d..dc75b3f0294 100644\n--- a/git.c\n+++ b/git.c\n@@ -5,6 +5,7 @@\n #include \"run-command.h\"\n #include \"alias.h\"\n #include \"shallow.h\"\n+#include \"gitcmd.h\"\n \n #define RUN_SETUP\t\t(1<<0)\n #define RUN_SETUP_GENTLY\t(1<<1)\n@@ -138,6 +139,7 @@ static int handle_options(const char ***argv, int *argc, int *envchanged)\n \n \twhile (*argc > 0) {\n \t\tconst char *cmd = (*argv)[0];\n+\t\tint show_usage = 0;\n \t\tif (cmd[0] != '-')\n \t\t\tbreak;\n \n@@ -247,22 +249,6 @@ static int handle_options(const char ***argv, int *argc, int *envchanged)\n \t\t\tsetenv(GIT_IMPLICIT_WORK_TREE_ENVIRONMENT, \"0\", 1);\n \t\t\tif (envchanged)\n \t\t\t\t*envchanged = 1;\n-\t\t} else if (!strcmp(cmd, \"-c\")) {\n-\t\t\tif (*argc < 2) {\n-\t\t\t\tfprintf(stderr, _(\"-c expects a configuration string\\n\" ));\n-\t\t\t\tusage(git_usage_string);\n-\t\t\t}\n-\t\t\tgit_config_push_parameter((*argv)[1]);\n-\t\t\t(*argv)++;\n-\t\t\t(*argc)--;\n-\t\t} else if (!strcmp(cmd, \"--config-env\")) {\n-\t\t\tif (*argc < 2) {\n-\t\t\t\tfprintf(stderr, _(\"no config key given for --config-env\\n\" ));\n-\t\t\t\tusage(git_usage_string);\n-\t\t\t}\n-\t\t\tgit_config_push_env((*argv)[1]);\n-\t\t\t(*argv)++;\n-\t\t\t(*argc)--;\n \t\t} else if (skip_prefix(cmd, \"--config-env=\", &cmd)) {\n \t\t\tgit_config_push_env(cmd);\n \t\t} else if (!strcmp(cmd, \"--literal-pathspecs\")) {\n@@ -295,19 +281,6 @@ static int handle_options(const char ***argv, int *argc, int *envchanged)\n \t\t\tset_alternate_shallow_file(the_repository, (*argv)[0], 1);\n \t\t\tif (envchanged)\n \t\t\t\t*envchanged = 1;\n-\t\t} else if (!strcmp(cmd, \"-C\")) {\n-\t\t\tif (*argc < 2) {\n-\t\t\t\tfprintf(stderr, _(\"no directory given for '%s' option\\n\" ), \"-C\");\n-\t\t\t\tusage(git_usage_string);\n-\t\t\t}\n-\t\t\tif ((*argv)[1][0]) {\n-\t\t\t\tif (chdir((*argv)[1]))\n-\t\t\t\t\tdie_errno(\"cannot change to '%s'\", (*argv)[1]);\n-\t\t\t\tif (envchanged)\n-\t\t\t\t\t*envchanged = 1;\n-\t\t\t}\n-\t\t\t(*argv)++;\n-\t\t\t(*argc)--;\n \t\t} else if (skip_prefix(cmd, \"--list-cmds=\", &cmd)) {\n \t\t\ttrace2_cmd_name(\"_query_\");\n \t\t\tif (!strcmp(cmd, \"parseopt\")) {\n@@ -322,6 +295,10 @@ static int handle_options(const char ***argv, int *argc, int *envchanged)\n \t\t\t} else {\n \t\t\t\texit(list_cmds(cmd));\n \t\t\t}\n+\t\t} else if (gity_handle_options(argv, argc, envchanged,\n+\t\t\t\t\t &show_usage)) {\n+\t\t\tif (show_usage)\n+\t\t\t\tusage(git_usage_string);\n \t\t} else {\n \t\t\tfprintf(stderr, _(\"unknown option: %s\\n\"), cmd);\n \t\t\tusage(git_usage_string);\ndiff --git a/gitcmd.c b/gitcmd.c\nnew file mode 100644\nindex 00000000000..0d8630ec2a4\n--- /dev/null\n+++ b/gitcmd.c\n@@ -0,0 +1,47 @@\n+#include \"cache.h\"\n+#include \"gitcmd.h\"\n+#include \"config.h\"\n+\n+int gity_handle_options(const char ***argv, int *argc,\n+\t\t\tint *envchanged, int *show_usage)\n+{\n+\tconst char *cmd = (*argv)[0];\n+\n+\tif (!strcmp(cmd, \"-c\")) {\n+\t\tif (*argc < 2) {\n+\t\t\tfprintf(stderr, _(\"-c expects a configuration string\\n\" ));\n+\t\t\tgoto usage;\n+\t\t}\n+\t\tgit_config_push_parameter((*argv)[1]);\n+\t\t(*argv)++;\n+\t\t(*argc)--;\n+\t} else if (!strcmp(cmd, \"--config-env\")) {\n+\t\tif (*argc < 2) {\n+\t\t\tfprintf(stderr, _(\"no config key given for --config-env\\n\" ));\n+\t\t\tgoto usage;\n+\t\t}\n+\t\tgit_config_push_env((*argv)[1]);\n+\t\t(*argv)++;\n+\t\t(*argc)--;\n+\t} else if (!strcmp(cmd, \"-C\")) {\n+\t\tif (*argc < 2) {\n+\t\t\tfprintf(stderr, _(\"no directory given for '%s' option\\n\" ), \"-C\");\n+\t\t\tgoto usage;\n+\t\t}\n+\t\tif ((*argv)[1][0]) {\n+\t\t\tif (chdir((*argv)[1]))\n+\t\t\t\tdie_errno(\"cannot change to '%s'\", (*argv)[1]);\n+\t\t\tif (envchanged)\n+\t\t\t\t*envchanged = 1;\n+\t\t}\n+\t\t(*argv)++;\n+\t\t(*argc)--;\n+\t} else {\n+\t\treturn 0;\n+\t}\n+\treturn 1;\n+usage:\n+\t*show_usage = 1;\n+\treturn 1;\n+}\n+\ndiff --git a/gitcmd.h b/gitcmd.h\nnew file mode 100644\nindex 00000000000..95c934b1500\n--- /dev/null\n+++ b/gitcmd.h\n@@ -0,0 +1,11 @@\n+#ifndef GITCMD_H\n+#define GITCMD_H\n+\n+/**\n+ * Handle options like in a \"git\"-y way, i.e. to emulate a command\n+ * that works like \"git\" itself. This is the part of handle_options()\n+ * that contrib/scalar needs from git.c.\n+ */\n+int gity_handle_options(const char ***argv, int *argc, int *envchanged,\n+\t\t\tint *show_usage);\n+#endif\n"},{"id":"447233","messageId":"5283a1b2-a31f-1657-1a67-cab6d9fec0ac@gmail.com","threadId":"57314","inReplyTo":"nycvar.QRO.7.76.6.2201281148310.347@tvgsbejvaqbjf.bet","subject":"Re: [PATCH] scalar: accept -C and -c options before the subcommand","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2022-01-28T19:37:22Z","receivedAt":"2022-01-28T19:37:28Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 1/28/2022 6:27 AM, Johannes Schindelin wrote:\n> Hi Stolee,\n> \n> On Thu, 27 Jan 2022, Derrick Stolee wrote:\n> \n>> The biggest benefits of using handle_options() is for other pre-command\n>> options such as --exec-path, which I use on a regular basis when testing\n>> new functionality.\n>>\n>> There are other options in handle_options() that might not be\n>> appropriate, or might be incorrect if we just make handle_options()\n>> non-static. For example, `scalar --list-cmds=parseopt` wouldn't show the\n>> scalar commands and would instead show the git commands.\n> \n> Right, and since `handle_options()` lives in the same file as `git`'s\n> `cmd_main()` function, we would not only have to disentangle options that\n> work only for `git` from those that would also work for `scalar`, but we\n> would have to extract the `handle_options()` function into a separate\n> file.\n\nI agree that these would be necessary steps.\n\n> But since I had a look at `handle_options()` anyway, I might just as well\n> summarize my insights about how applicable the supported options are for\n> `scalar` here:\n> \n> # Beneficial\n> \n> -c =\n> --config-env =\n> -C \n> \n> \tSince I added support for these (except for the long form\n> \t`--config-env` that I actually only learned while researching this\n> \temail), it is obvious that I'd like `scalar` to support them.\n>\n> # Won't hurt\n\nThese \"Won't hurt\" items look to me as \"they probably don't matter, but\nit would be nice if \"scalar