All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kyle Meyer <kyle@kyleam.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org
Subject: [PATCH v2] send-email: relay '-v N' to format-patch
Date: Sat, 26 Nov 2022 15:21:23 -0500	[thread overview]
Message-ID: <87edtp5uws.fsf@kyleam.com> (raw)
In-Reply-To: <87k03j54aj.fsf@kyleam.com>

Kyle Meyer writes:

> Junio C Hamano writes:
>
>> The right solution for allowing "-v 3" given to "format-patch" I think
>> is to make send-email understand it and pass that through.  The
>> presence of both ("validate" => \$validate) and ("v" =>
>> \$reroll_count) in the GetOptions() argument would prevent "-v" to be
>> taken as "--validate" while still allowing "--val" to be used as an
>> abbrevatiion, no?
>
> I'd think that would work, yes.  I'll look more into going this route.
>
> With that approach, there are other cases of abbreviation intercepting
> valid format patch options.  [...]

Here's a patch handling the -v case.  I don't plan on working on a more
complete fix for the other cases (as I mentioned before, I don't use
send-email to drive format-patch), but in my opinion the -v fix by
itself is still valuable.

-- >8 --
Subject: [PATCH v2] send-email: relay '-v N' to format-patch

send-email relays unrecognized arguments to its format-patch call.
Passing '-v N' leads to an error because -v is consumed as
send-email's --validate.  For example,

  git send-email -v 3 @{u}

fails with

  fatal: ambiguous argument '3': unknown revision or path not in the
  working tree.  [...]

To prevent this, add the short --reroll-count option to send-email's
main option list and explicitly provide it to the format-patch call.

There other format-patch options that send-email doesn't relay
properly, including at least -n, -N, and the diff option -D.  Punt on
these because dealing with them is more complicated:

 * they would require configuring send-email to not ignore option case

 * send-email makes three GetOptions() calls with different sets of
   options, the last being the main set of options.  Unlike -v, which
   is consumed by the last GetOptions call, the -n, -N, and -D options
   are consumed as abbreviations by the earlier calls.

Signed-off-by: Kyle Meyer <kyle@kyleam.com>
---
 git-send-email.perl   | 9 ++++++++-
 t/t9001-send-email.sh | 6 ++++++
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index 5861e99a6e..07f2a0cbea 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -220,6 +220,10 @@ sub format_2822_time {
 my $force = 0;
 my $dump_aliases = 0;
 
+# Variables to prevent short format-patch options from being captured
+# as abbreviated send-email options
+my $reroll_count;
+
 # Handle interactive edition of files.
 my $multiedit;
 my $editor;
@@ -542,6 +546,7 @@ sub config_regexp {
 		    "batch-size=i" => \$batch_size,
 		    "relogin-delay=i" => \$relogin_delay,
 		    "git-completion-helper" => \$git_completion_helper,
+		    "v=s" => \$reroll_count,
 );
 $rc = GetOptions(%options);
 
@@ -782,7 +787,9 @@ sub is_format_patch_arg {
 	die __("Cannot run git format-patch from outside a repository\n")
 		unless $repo;
 	require File::Temp;
-	push @files, $repo->command('format-patch', '-o', File::Temp::tempdir(CLEANUP => 1), @rev_list_opts);
+	push @files, $repo->command('format-patch', '-o', File::Temp::tempdir(CLEANUP => 1),
+				    defined $reroll_count ? ('-v', $reroll_count) : (),
+				    @rev_list_opts);
 }
 
 @files = handle_backup_files(@files);
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 01c74b8b07..152bd2c697 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -2334,6 +2334,12 @@ test_expect_success $PREREQ 'test that send-email works outside a repo' '
 		"$(pwd)/0001-add-main.patch"
 '
 
+test_expect_success $PREREQ 'send-email relays -v 3 to format-patch' '
+	test_when_finished "rm -f out" &&
+	git send-email --dry-run -v 3 -1 >out &&
+	grep "PATCH v3" out
+'
+
 test_expect_success $PREREQ 'test that sendmail config is rejected' '
 	test_config sendmail.program sendmail &&
 	test_must_fail git send-email \

base-commit: e7e5c6f715b2de7bea0d39c7d2ba887335b40aa0
-- 
2.38.1


  reply	other threads:[~2022-11-26 20:21 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-24  2:00 [PATCH] send-email: disable option auto-abbreviation Kyle Meyer
2022-11-25  7:11 ` Junio C Hamano
2022-11-25 17:31   ` Kyle Meyer
2022-11-26 20:21     ` Kyle Meyer [this message]
2022-11-27  1:25       ` [PATCH v2] send-email: relay '-v N' to format-patch Junio C Hamano
2022-11-28 12:34         ` Ævar Arnfjörð Bjarmason
2022-11-28  9:41       ` Junio C Hamano

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=87edtp5uws.fsf@kyleam.com \
    --to=kyle@kyleam.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.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.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.