git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Patrick Steinhardt <ps@pks.im>
Cc: git@vger.kernel.org
Subject: Re: [PATCH 3/3] revision: handle pseudo-opts in `--stdin` mode
Date: Wed, 14 Jun 2023 08:56:00 -0700	[thread overview]
Message-ID: <xmqq352uyr5b.fsf@gitster.g> (raw)
In-Reply-To: <457591712799719f23fa47e59d5a3c1cd497e802.1686744685.git.ps@pks.im> (Patrick Steinhardt's message of "Wed, 14 Jun 2023 14:18:42 +0200")

Patrick Steinhardt <ps@pks.im> writes:

>  		if (sb.buf[0] == '-') {
> +			const char *argv[2] = { sb.buf, NULL };
> +
>  			if (!strcmp(sb.buf, "--")) {
>  				seen_dashdash = 1;
>  				break;
>  			}
>  
> -			die("options not supported in --stdin mode");
> +			if (handle_revision_pseudo_opt(revs, argv, flags))
> +				continue;
> +
> +			die(_("option '%s' not supported in --stdin mode"), sb.buf);
>  		}

The reason why we had the double-dash checking inside this block is
because we rejected any dashed options other than double-dash, but
with this step, that is no longer true, so it may make more sense to
move the "seen_dashdash" code outside the block.  Also unlike the
command line options that allows "--end-of-options" marker to allow
tweaking how a failing handle_revision_arg() call is handled, this
codepath does not seem to have any matching provision.  In the
original code, which did not accept any options, it was perfectly
understandable that it was unaware of "--end-of-options", but now we
do allow dashed options, shouldn't we be supporting it here, too?

Thanks.

  reply	other threads:[~2023-06-14 15:56 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-14 12:18 [PATCH 0/3] revision: handle pseudo-opts in `--stdin` mode Patrick Steinhardt
2023-06-14 12:18 ` [PATCH 1/3] revision: reorder `read_revisions_from_stdin()` Patrick Steinhardt
2023-06-14 16:01   ` Junio C Hamano
2023-06-14 12:18 ` [PATCH 2/3] revision: small readability improvement for reading from stdin Patrick Steinhardt
2023-06-14 16:03   ` Junio C Hamano
2023-06-14 12:18 ` [PATCH 3/3] revision: handle pseudo-opts in `--stdin` mode Patrick Steinhardt
2023-06-14 15:56   ` Junio C Hamano [this message]
2023-06-15 14:25     ` Patrick Steinhardt
2023-06-15 14:39 ` [PATCH v2 0/3] " Patrick Steinhardt
2023-06-15 14:39   ` [PATCH v2 1/3] revision: reorder `read_revisions_from_stdin()` Patrick Steinhardt
2023-06-15 14:39   ` [PATCH v2 2/3] revision: small readability improvement for reading from stdin Patrick Steinhardt
2023-06-15 14:39   ` [PATCH v2 3/3] revision: handle pseudo-opts in `--stdin` mode Patrick Steinhardt

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

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqq352uyr5b.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=ps@pks.im \
    /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.
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 NNTP newsgroup(s).