From: Johannes Schindelin <Johannes.Schindelin@gmx.de>
To: Taylor Blau <me@ttaylorr.com>
Cc: Johannes Schindelin via GitGitGadget <gitgitgadget@gmail.com>,
git@vger.kernel.org
Subject: Re: [PATCH] scalar: accept -C and -c options before the subcommand
Date: Fri, 28 Jan 2022 12:43:44 +0100 (CET) [thread overview]
Message-ID: <nycvar.QRO.7.76.6.2201281227330.347@tvgsbejvaqbjf.bet> (raw)
In-Reply-To: <YfG0ybYOCwDzlbi5@nand.local>
Hi Taylor,
On Wed, 26 Jan 2022, Taylor Blau wrote:
> On Wed, Jan 26, 2022 at 11:15:29AM +0000, Johannes Schindelin via GitGitGadget wrote:
> > From: Johannes Schindelin <johannes.schindelin@gmx.de>
> >
> > The `git` executable has these two very useful options:
> >
> > -C <directory>:
> > switch to the specified directory before performing any actions
> >
> > -c <key>=<value>:
> > temporarily configure this setting for the duration of the
> > specified scalar subcommand
> >
> > With this commit, we teach the `scalar` executable the same trick.
> >
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> > ---
> > scalar: accept -C and -c options
> >
> > This makes the scalar command a bit more handy by offering the same -c
> > <key>=<value> and -C <directory> options as the git command.
> >
> > Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1130%2Fdscho%2Fscalar-c-and-C-v1
> > Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1130/dscho/scalar-c-and-C-v1
> > Pull-Request: https://github.com/gitgitgadget/git/pull/1130
> >
> > contrib/scalar/scalar.c | 22 +++++++++++++++++++++-
> > contrib/scalar/scalar.txt | 10 ++++++++++
> > 2 files changed, 31 insertions(+), 1 deletion(-)
> >
> > diff --git a/contrib/scalar/scalar.c b/contrib/scalar/scalar.c
> > index 1ce9c2b00e8..7db2a97416e 100644
> > --- a/contrib/scalar/scalar.c
> > +++ b/contrib/scalar/scalar.c
> > @@ -808,6 +808,25 @@ int cmd_main(int argc, const char **argv)
> > struct strbuf scalar_usage = STRBUF_INIT;
> > int i;
> >
> > + while (argc > 1 && *argv[1] == '-') {
> > + if (!strcmp(argv[1], "-C")) {
> > + if (argc < 3)
> > + die(_("-C requires a <directory>"));
> > + if (chdir(argv[2]) < 0)
> > + die_errno(_("could not change to '%s'"),
> > + argv[2]);
> > + argc -= 2;
> > + argv += 2;
> > + } else if (!strcmp(argv[1], "-c")) {
> > + if (argc < 3)
> > + die(_("-c requires a <key>=<value> argument"));
> > + git_config_push_parameter(argv[2]);
> > + argc -= 2;
> > + argv += 2;
> > + } else
> > + break;
> > + }
> > +
>
> All looks right to me based on a cursory scan. It's too bad that we have
> to copy this code from git.c::handle_options().
It's only a dozen lines, though, and they are pretty stable, so I doubt
that we risk divergent copied code.
> Could we call handle_options() (assuming that it was available to
> Scalar's compilation unit) instead? I'm not sure if that's a naive
> question or not, but it might be nice to explain it out in the commit
> message in case other reviewers have the same question that I did.
I just responded to Stolee elsewhere in this thread with a lengthy
analysis of the options, and the conclusion that it would not be worth the
effort to refactor `handle_options()`.
> On a more practical note: is there an easy way to test this?
It would be pretty easy to test `-C`:
git init sub &&
scalar -C sub register &&
[... verify that `sub/` is now a Scalar repository ...]
For `-c`, we would need to configure something parsed by
`git_default_config()` that would influence what `scalar register` does,
then verify that. Or even better, use a config setting that is in the
"Optional" section of `set_recommended_config()`, i.e. it will refuse to
override an already-configured value. Something like `status.aheadBehind`.
I added this:
-- snip --
diff --git a/contrib/scalar/t/t9099-scalar.sh b/contrib/scalar/t/t9099-scalar.sh
index 2e1502ad45e..89781568f43 100755
--- a/contrib/scalar/t/t9099-scalar.sh
+++ b/contrib/scalar/t/t9099-scalar.sh
@@ -85,4 +85,12 @@ test_expect_success 'scalar delete with enlistment' '
test_path_is_missing cloned
'
+test_expect_success 'scalar supports -c/-C' '
+ test_when_finished "scalar delete sub" &&
+ git init sub &&
+ scalar -C sub -c status.aheadBehind=bogus register &&
+ test -z "$(git -C sub config --local status.aheadBehind)" &&
+ test true = "$(git -C sub config core.preloadIndex)"
+'
+
test_done
-- snap --
Ciao,
Dscho
next prev parent reply other threads:[~2022-01-28 11:43 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-01-26 11:15 [PATCH] scalar: accept -C and -c options before the subcommand Johannes Schindelin via GitGitGadget
2022-01-26 20:53 ` Taylor Blau
2022-01-28 11:43 ` Johannes Schindelin [this message]
2022-01-27 2:55 ` Ævar Arnfjörð Bjarmason
2022-01-27 14:46 ` Derrick Stolee
2022-01-28 11:27 ` Johannes Schindelin
2022-01-28 18:21 ` Ævar Arnfjörð Bjarmason
2022-01-28 19:52 ` Derrick Stolee
2022-01-29 6:39 ` Ævar Arnfjörð Bjarmason
2022-01-28 19:37 ` Derrick Stolee
2022-01-28 18:05 ` Junio C Hamano
2022-01-28 19:38 ` Derrick Stolee
2022-01-28 14:31 ` [PATCH v2] " Johannes Schindelin via GitGitGadget
2022-01-28 19:40 ` Derrick Stolee
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=nycvar.QRO.7.76.6.2201281227330.347@tvgsbejvaqbjf.bet \
--to=johannes.schindelin@gmx.de \
--cc=git@vger.kernel.org \
--cc=gitgitgadget@gmail.com \
--cc=me@ttaylorr.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).