From: Taylor Blau <me@ttaylorr.com>
To: Johannes Schindelin via GitGitGadget <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org, Johannes Schindelin <johannes.schindelin@gmx.de>
Subject: Re: [PATCH] scalar: accept -C and -c options before the subcommand
Date: Wed, 26 Jan 2022 15:53:29 -0500 [thread overview]
Message-ID: <YfG0ybYOCwDzlbi5@nand.local> (raw)
In-Reply-To: <pull.1130.git.1643195729608.gitgitgadget@gmail.com>
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().
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.
On a more practical note: is there an easy way to test this? Like I
said, I'm not necessarily worried about the code you wrote, just that it
seems like this sort of thing *should* be easy enough to test. It's fine
if there isn't, too, but again as somebody new to this area some
explanation would be helpful.
Thanks,
Taylor
next prev parent reply other threads:[~2022-01-26 20:53 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 [this message]
2022-01-28 11:43 ` Johannes Schindelin
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=YfG0ybYOCwDzlbi5@nand.local \
--to=me@ttaylorr.com \
--cc=git@vger.kernel.org \
--cc=gitgitgadget@gmail.com \
--cc=johannes.schindelin@gmx.de \
/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).