From: Junio C Hamano <hidden> Date: 2021-08-20 20:17:15
Thiago Perrotta [off-list ref] writes:
Rationale: Currently all git built-in commands print a newline upon upon git
<cmd> --git-completion-helper. Therefore git-send-email should follow suit for
consistency.
"upon upon".
You do not need to start with "Rationale", because one of the things
you need to have in any proposed log message is a justification for
the change. You also do not need "Currently" in our log message, as
the convention here is to state the status quo in the present tense,
point out what's wrong there (or leave it unsaid and implied by the
description of the current state, if it is obvious), state the
approach to correct what's wrong, and finally give an order to the
codebase to "become like so".
Unlike other Git subcommands, "git send-email" leaves its
output an incomplete line when "--git-completion-helper" is
asked. Be consistent by terminating the message with LF
here.
or something like that.
I do not know which style is preferred among
(1) print something;
print "\n";
(2) print something, "\n";
(3) print something . "\n";
but other than that, the goal and the implementation both sound
sensible (the second one is what I'd be writing if I were doing this
change myself, FWIW).
Thanks.
Unlike other Git subcommands, "git send-email" leaves its output an
incomplete line when "--git-completion-helper" is asked. Be consistent
by terminating the message with LF here.
Signed-off-by: Thiago Perrotta <redacted>
---
git-send-email.perl | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
git-send-email(1) does not mention that "git format-patch" options are
accepted. Augment SYNOPSIS and DESCRIPTION to mention it.
Signed-off-by: Thiago Perrotta <redacted>
---
Documentation/git-send-email.txt | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
@@ -9,7 +9,8 @@ git-send-email - Send a collection of patches as emails SYNOPSIS -------- [verse]-'git send-email' [<options>] <file|directory|rev-list options>...+'git send-email' [<options>] <file|directory>...+'git send-email' [<options>] <format-patch options> 'git send-email' --dump-aliases
@@ -19,7 +20,8 @@ Takes the patches given on the command line and emails them out. Patches can be specified as files, directories (which will send all files in the directory), or directly as a revision list. In the last case, any format accepted by linkgit:git-format-patch[1] can-be passed to git send-email.+be passed to git send-email, and options understood by+linkgit:git-format-patch[1] can be passed. The header of the email is configurable via command-line options. If not specified on the command line, the user will be prompted with a ReadLine
@@ -19,7 +20,8 @@ Takes the patches given on the command line and emails them out. Patches can be specified as files, directories (which will send all files in the directory), or directly as a revision list. In the last case, any format accepted by linkgit:git-format-patch[1] can-be passed to git send-email.+be passed to git send-email, and options understood by+linkgit:git-format-patch[1] can be passed.
Better say "..., as well as options understood by
linkgit:git-format-patch[1].".
--
An old man doll... just what I always wanted! - Clara
13374987dd (completion: use __gitcomp_builtin for format-patch,
2018-11-03) mentions
that keeping these in the shell script help with caching and that
moving them to perl would
be better done so that the list can be maintained programmatically
instead of manually.
FWIW it is missing several options (ex: batch-size, {cc,to}-cover,
{8bit,compose}-encoding)
Carlo
"git send-email --git-completion-helper" only prints "format-patch"
flags. Make it print "send-email" flags as well.
Add a completion test for "send-email --validate", a send-email option.
Signed-off-by: Thiago Perrotta <redacted>
---
Hi Carlos,
Good catch. Initially I had only moved the options from the script.
I am working on another patch (attached) that exhaustively adds all
options. At this time, it's still manual, I agree that generating the
flags programatically would be even better, but I didn't want to
complicate this series too much.
Let me know how you would like me to proceed here, I see 3 options:
1. drop the second part of the patch (v3 2/3) completely, only keep the
other ones (1/3 and 3/3), which just adds the newline and touches the
man page - leave automated flags for another series
2. send v4 with the attached - which exhaustively adds all send-email
options to the perl script, while removing the completions from the bash
completion script
3. send v4 with the attached and do not touch the current bash
completion script (keeping caching as is)
contrib/completion/git-completion.bash | 11 +----
git-send-email.perl | 62 ++++++++++++++++++++++++++
t/t9902-completion.sh | 3 ++
3 files changed, 66 insertions(+), 10 deletions(-)
From: Carlo Arenas <hidden> Date: 2021-09-07 01:28:35
On Mon, Sep 6, 2021 at 5:16 PM Thiago Perrotta [off-list ref] wrote:
Let me know how you would like me to proceed here, I see 3 options:
1. drop the second part of the patch (v3 2/3) completely, only keep the
other ones (1/3 and 3/3), which just adds the newline and touches the
man page - leave automated flags for another series
This seems like the most straightforward and quick way out, but I was
really hoping you could take the time to instead do the move to perl
with the programmatic option, which is long overdue anyway.
If you go with 1, then 2 and an improved 4 could be done as a follow up
Carlo
"git send-email" completion (bash, zsh) is inconsistent, its flags are
split into both git-completion.bash and git-send-email.perl, and it only
emits format-patch flags.
Make shell completion uniform, centralizing completion
options on git-send-email.perl.
Make "git send-email --git-completion-helper" properly emit send-email
specific options.
Additionally, update git-send-email(1) man page to explicitly
mention format-patch options.
Differences from V3:
- Incorporate Carlo Arenas' code suggestions, adding all flags
exhaustively.
- Incorporate Bagas Sanjaya's suggestion.
Differences from V2:
- Incorporate Junio's code suggestions.
- Follow proper conventions for git commit messages.
Carlo suggests to generate the flags programatically from the perl
script. I am looking into this and already have a proof-of-concept
working and plan to submit it as a separate patch series. I would like
to get this series checked in first though.
Thiago Perrotta (3):
send-email: terminate --git-completion-helper with LF
send-email: move bash completions to core script
send-email docs: add format-patch options
Documentation/git-send-email.txt | 6 ++-
contrib/completion/git-completion.bash | 11 +----
git-send-email.perl | 64 +++++++++++++++++++++++++-
t/t9902-completion.sh | 3 ++
4 files changed, 71 insertions(+), 13 deletions(-)
--
2.33.0
Unlike other Git subcommands, "git send-email" leaves its output an
incomplete line when "--git-completion-helper" is asked. Be consistent
by terminating the message with LF here.
Signed-off-by: Thiago Perrotta <redacted>
---
git-send-email.perl | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
git-send-email(1) does not mention that "git format-patch" options are
accepted. Augment SYNOPSIS and DESCRIPTION to mention it.
Signed-off-by: Thiago Perrotta <redacted>
---
Documentation/git-send-email.txt | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
@@ -9,7 +9,8 @@ git-send-email - Send a collection of patches as emails SYNOPSIS -------- [verse]-'git send-email' [<options>] <file|directory|rev-list options>...+'git send-email' [<options>] <file|directory>...+'git send-email' [<options>] <format-patch options> 'git send-email' --dump-aliases
@@ -19,7 +20,8 @@ Takes the patches given on the command line and emails them out. Patches can be specified as files, directories (which will send all files in the directory), or directly as a revision list. In the last case, any format accepted by linkgit:git-format-patch[1] can-be passed to git send-email.+be passed to git send-email, as well as options understood by+linkgit:git-format-patch[1]. The header of the email is configurable via command-line options. If not specified on the command line, the user will be prompted with a ReadLine
On Tue, Sep 21 2021, Thiago Perrotta wrote:
Note: using --in-reply-to to the previous version in "git format-patch"
helps keep track of the context.
Carlo suggests to generate the flags programatically from the perl
script. I am looking into this and already have a proof-of-concept
working and plan to submit it as a separate patch series. I would like
to get this series checked in first though.
Isn't this just:
my @params = <getopts list>;
GetOptions(@params);
And then doing some light parsing/slicinng of the @params list to get
the keys you've duplicated here?
I for one would much prefer to see that go in right away than the churn
of first hardcoding these, then removing the hardcoding etc.
"git send-email" completion (bash, zsh) is inconsistent, its flags are
split into both git-completion.bash and git-send-email.perl, and it only
emits format-patch flags.
Make shell completion uniform, centralizing completion
options on git-send-email.perl.
Make "git send-email --git-completion-helper" properly emit send-email
specific options, generating them programmatically.
Additionally, update git-send-email(1) man page to explicitly
mention format-patch options.
Differences from V4:
Incorporate Carlo Arenas' and Ævar Arnfjörð Bjarmason's suggestion to
programatically generate the flags.
I tried to be concise whilst preserving readability, hopefully it's
straightforward to parse the current implementation.
Reviewers of previous versions:
- Bagas Sanjaya
- Carlo Arenas
- Junio Hamano
- Ævar Arnfjörð Bjarmason
Ævar wrote earlier:
Note: using --in-reply-to to the previous version in "git format-patch"
helps keep track of the context.
Thanks for the heads up, I am slowly getting used to this email
workflow, this is my first contribution. Hopefully I got it right this
time.
Isn't this just:
my @params = <getopts list>;
GetOptions(@params);
Let me know if the regex approach I went with is OK. You seem to have
suggested me to do something with the `GetOptions` module, but I'm
afraid I only know the basics of Perl.
I tried to do something like `GetOptions($USAGE)` but it didn't quite
work (clearly I have no idea how to do that :P). If you have something
specific in mind, I'd appreciate if you could send a small patch back
that I can incorporate. Otherwise, either way, the current regex
approach isn't too horrible and seems to be reasonably reliable.
Thiago Perrotta (3):
send-email: terminate --git-completion-helper with LF
send-email: programmatically generate bash completions
send-email docs: add format-patch options
Documentation/git-send-email.txt | 6 ++++--
contrib/completion/git-completion.bash | 11 +----------
git-send-email.perl | 21 +++++++++++++++++----
t/t9902-completion.sh | 3 +++
4 files changed, 25 insertions(+), 16 deletions(-)
--
2.33.0
Unlike other Git subcommands, "git send-email" leaves its output an
incomplete line when "--git-completion-helper" is asked. Be consistent
by terminating the message with LF here.
Signed-off-by: Thiago Perrotta <redacted>
---
git-send-email.perl | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
"git send-email --git-completion-helper" only prints "format-patch"
flags. Make it print "send-email" flags as well, generating them
programmatically from the usage.
Introduce a uniq subroutine, otherwise --dump-aliases would show
up twice in the flags.
Add a completion test for "send-email --validate", a send-email option.
Signed-off-by: Thiago Perrotta <redacted>
---
contrib/completion/git-completion.bash | 11 +----------
git-send-email.perl | 21 +++++++++++++++++----
t/t9902-completion.sh | 3 +++
3 files changed, 21 insertions(+), 14 deletions(-)
git-send-email(1) does not mention that "git format-patch" options are
accepted. Augment SYNOPSIS and DESCRIPTION to mention it.
Signed-off-by: Thiago Perrotta <redacted>
---
Documentation/git-send-email.txt | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
@@ -9,7 +9,8 @@ git-send-email - Send a collection of patches as emails SYNOPSIS -------- [verse]-'git send-email' [<options>] <file|directory|rev-list options>...+'git send-email' [<options>] <file|directory>...+'git send-email' [<options>] <format-patch options> 'git send-email' --dump-aliases
@@ -19,7 +20,8 @@ Takes the patches given on the command line and emails them out. Patches can be specified as files, directories (which will send all files in the directory), or directly as a revision list. In the last case, any format accepted by linkgit:git-format-patch[1] can-be passed to git send-email.+be passed to git send-email, as well as options understood by+linkgit:git-format-patch[1]. The header of the email is configurable via command-line options. If not specified on the command line, the user will be prompted with a ReadLine
Is <format-patch options> optional? If so, we can say [<format-patch
options>].
no; as Junio explained [1] this omission is intentional while the
rev-list options that
got cut to make space are not and are more relevant.
IMHO leaving [<options>] to imply ALL options (that also include diff
options, for example) is better
Carlo
[1] https://lore.kernel.org/git/xmqqh7fjuaar.fsf@gitster.g/
no; as Junio explained [1] this omission is intentional while the
rev-list options that
got cut to make space are not and are more relevant.
IMHO leaving [<options>] to imply ALL options (that also include diff
options, for example) is better
Quoting what you linked:
The program works in two majorly different modes. It either takes
* message files that are already proof-read and copy-edited from
the filesystem and sends them out, or
* format-patch options to generate message files out of the commits
and send them out without any proofreading.
I think we need to again modify SYNOPSIS and DESCRIPTIONS to highlight
above fact:
---- 8> ----
diff --git a/Documentation/git-send-email.txt
b/Documentation/git-send-email.txt
index 3db4eab4ba..6002ca1a02 100644
@@ -9,17 +9,19 @@ git-send-email - Send a collection of patches as emails SYNOPSIS -------- [verse]-'git send-email' [<options>] <file|directory|rev-list options>...+'git send-email' [<options>] <file|directory>...+'git send-email' [<options>] <revision list>... 'git send-email' --dump-aliases DESCRIPTION ------------Takes the patches given on the command line and emails them out.-Patches can be specified as files, directories (which will send all-files in the directory), or directly as a revision list. In the-last case, any format accepted by linkgit:git-format-patch[1] can-be passed to git send-email.+In the first form, take the patches in <file> or <directory> and+emails them out. In the second form, generate patches from specified+<revision list> (it can be revision range or explicit list of+revisions from linkgit:git-rev-list[1]), then emails them out.+<options> can also include options understood by+linkgit:git-format-patch[1] if the second form is specified. The header of the email is configurable via command-line options. If not specified on the command line, the user will be prompted with a ReadLine
--
An old man doll... just what I always wanted! - Clara
From: Carlo Arenas <hidden> Date: 2021-09-24 06:56:57
On Thu, Sep 23, 2021 at 11:19 PM Bagas Sanjaya [off-list ref] wrote:
On 24/09/21 11.53, Carlo Arenas wrote:
quoted
no; as Junio explained [1] this omission is intentional while the
rev-list options that
got cut to make space are not and are more relevant.
IMHO leaving [<options>] to imply ALL options (that also include diff
options, for example) is better
Quoting what you linked:
quoted
The program works in two majorly different modes. It either takes
* message files that are already proof-read and copy-edited from
the filesystem and sends them out, or
* format-patch options to generate message files out of the commits
and send them out without any proofreading.
I think we need to again modify SYNOPSIS and DESCRIPTIONS to highlight
above fact:
Bagas,
This is much better; can you SOB and help Thiago import it into his
tree so it can be included in the next reroll?
Thiago,
Let's wait a little while to collect more feedback on patch2 before
sending it, though
Carlo
Thanks for the heads up, I am slowly getting used to this email
workflow, this is my first contribution. Hopefully I got it right this
time.
quoted
Isn't this just:
quoted
my @params = <getopts list>;
GetOptions(@params);
Let me know if the regex approach I went with is OK. You seem to have
suggested me to do something with the `GetOptions` module, but I'm
afraid I only know the basics of Perl.
I tried to do something like `GetOptions($USAGE)` but it didn't quite
work (clearly I have no idea how to do that :P). If you have something
specific in mind, I'd appreciate if you could send a small patch back
that I can incorporate. Otherwise, either way, the current regex
approach isn't too horrible and seems to be reasonably reliable.
I meant something like the below patch, feel free to incorporate it if
you'd like with my signed-off-by, i.e. there's no reason to parse the
usage message, or hardcode another set of options, we've got it right
there as structured program data being fed to the GetOptions() function.
All we need to do is to assign that to a hash, and use it both for
emitting the help and to call GetOptions().
What I have doesn't *quite* work, i.e. the --git-completion-helper
expects "--foo=" I think for things that are "foo=s" in perl, so the
regex needs adjusting, but that should be an easy addition on top.
-- >8 --
@@ -114,8 +114,18 @@ sub usage {}subcompletion_helper{-printGit::command('format-patch','--git-completion-helper');-exit(0);+my($options)=@_;++my@gse_options=sortmap{+"--$_"+}map{+s/(?:[:=][si]|!)$//;+split/\|/,$_;+}keys%$options;+my@fpa_options=Git::command('format-patch','--git-completion-helper');+my@options=sort@gse_options,@fpa_options;+print"@options\n";+exit(0);}# most mail servers generate the Date: header, but not all...
@@ -449,7 +459,7 @@ sub config_regexp {usage()unless$rc;die__("--dump-aliases incompatible with other options\n")if!$helpand$dump_aliasesand@ARGV;-$rc=GetOptions(+my%options=("sender|from=s"=>\$sender,"in-reply-to=s"=>\$initial_in_reply_to,"reply-to=s"=>\$reply_to,
@@ -508,7 +518,8 @@ sub config_regexp {"batch-size=i"=>\$batch_size,"relogin-delay=i"=>\$relogin_delay,"git-completion-helper"=>\$git_completion_helper,-);+);+$rc=GetOptions(%options);# Munge any "either config or getopt, not both" variablesmy@initial_to=@getopt_to?@getopt_to:($no_to?():@config_to);
@@ -516,7 +527,7 @@ sub config_regexp {my@initial_bcc=@getopt_bcc?@getopt_bcc:($no_bcc?():@config_bcc);usage()if$help;-completion_helper()if$git_completion_helper;+completion_helper(\%options)if$git_completion_helper;unless($rc){usage();}
On Fri, 24 Sept 2021 at 16:07, Ævar Arnfjörð Bjarmason [off-list ref] wrote:
I meant something like the below patch, feel free to incorporate it if
you'd like with my signed-off-by, i.e. there's no reason to parse the
usage message, or hardcode another set of options, we've got it right
there as structured program data being fed to the GetOptions() function.
All we need to do is to assign that to a hash, and use it both for
emitting the help and to call GetOptions().
What I have doesn't *quite* work, i.e. the --git-completion-helper
expects "--foo=" I think for things that are "foo=s" in perl, so the
regex needs adjusting, but that should be an easy addition on top.
1)
Thanks Ævar, I get the gist of it. Your approach revealed a few issues
with the current usage string:
The following options exist in GetOptions but not in the usage string:
--git-completion-helper
--no-signed-off-cc
--sender
--signed-off-cc
Out of these, I'd argue --git-completion-helper is intentionally omitted,
however --sender and --signed-off-cc were overlooked.
2)
Also, your patch misses --dump-aliases and --identity; that's because
they are in other GetOptions functions in the file.
The two obvious possibilities here are either (i) hard-code them directly, i.e.:
-my @options = sort @gse_options, @fpa_options;
+my @options = sort @gse_options, @fpa_options, "--dump-aliases", "--sender";
or (ii) refactor the other two GetOptions like you did in your patch,
so that `sub completion_helper` ends up receiving all three hashes
(or maybe a single hash as a result of all three merged).
Any preference between (i) or (ii)? I am leaning towards (i).
3)
Finally, I noticed that "sort @gse_options, @fpa_options" doesn't
really sort fpa_options.
If sorting is really intended, it would be better to modify the source
of format-patch to
emit sorted output.
Otherwise, we may as well leave it untouched. AFAIK from a completion
perspective it
seems that it doesn't matter: both bash and zsh emit `git format-patch
--<TAB>` sorted
today, even though the output of `git format-patch
--git-completion-helper` isn't sorted.
The only benefit of sorting I see would be to deduplicate ('uniq') flags.
Do you agree with this rationale?
Either way, let me know whether or not it's preferable to sort.
I'll probably sort `send-email` options anyway just to deduplicate a
few flags such as --to-cover,
but `format-patch` could remain as is.
I'll wait for replies before sending another patch (on top of your
original one).
Unlike other Git subcommands, "git send-email" leaves its output an
incomplete line when "--git-completion-helper" is asked. Be consistent
by terminating the message with LF here.
Signed-off-by: Thiago Perrotta <redacted>
---
git-send-email.perl | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Differences from V5:
Addresses original Ævar's patch:
- Effectively including all combined options, which ends up adding
--dump-aliases and --identity.
- Manually excluded --h and --git-completion-helper. This is consistent
with "git format-patch --git-completion-helper".
- Completely dropped sorting, because it seems the current git
subcommands don't really care about it (I verified "git format-patch"
and "git apply" only).
As mentioned earlier, the following issue remains open:
"The following options exist in GetOptions but not in the current
usage string:
--no-signed-off-cc --sender --signed-off-cc"
I wouldn't mind adding a fourth patch to this series that fixes the
current usage, just let me know which wording you'd like me to add.
Finally, I will note that I haven't touched the third patch of this
series (git-send-email.txt) because it is not obvious to me which
direction we're taking. My understanding is that Junio prefers to leave
things as is, but let me know if I should change it to e.g. Carlos' or
Bagas' version.
Thiago Perrotta (3):
send-email: terminate --git-completion-helper with LF
send-email: programmatically generate bash completions
send-email docs: add format-patch options
Documentation/git-send-email.txt | 6 ++--
contrib/completion/git-completion.bash | 11 +------
git-send-email.perl | 43 +++++++++++++++++++-------
t/t9902-completion.sh | 3 ++
4 files changed, 40 insertions(+), 23 deletions(-)
--
2.33.0
git-send-email(1) does not mention that "git format-patch" options are
accepted. Augment SYNOPSIS and DESCRIPTION to mention it.
Signed-off-by: Thiago Perrotta <redacted>
---
Documentation/git-send-email.txt | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
@@ -9,7 +9,8 @@ git-send-email - Send a collection of patches as emails SYNOPSIS -------- [verse]-'git send-email' [<options>] <file|directory|rev-list options>...+'git send-email' [<options>] <file|directory>...+'git send-email' [<options>] <format-patch options> 'git send-email' --dump-aliases
@@ -19,7 +20,8 @@ Takes the patches given on the command line and emails them out. Patches can be specified as files, directories (which will send all files in the directory), or directly as a revision list. In the last case, any format accepted by linkgit:git-format-patch[1] can-be passed to git send-email.+be passed to git send-email, as well as options understood by+linkgit:git-format-patch[1]. The header of the email is configurable via command-line options. If not specified on the command line, the user will be prompted with a ReadLine
"git send-email --git-completion-helper" only prints "format-patch"
flags. Make it print "send-email" flags as well, generating them
programmatically from the usage.
Extract flags from the three existing `GetOptions`.
Introduce a uniq subroutine, otherwise --cc-cover, --to-cover and other
flags would show up twice.
Remove two extraneous flags: --h and --git-completion-helper.
Add a completion test for "send-email --validate", a send-email flag.
Signed-off-by: Thiago Perrotta <redacted>
Based-on-patch-by: Ævar Arnfjörð Bjarmason [off-list ref]
---
contrib/completion/git-completion.bash | 11 +------
git-send-email.perl | 43 +++++++++++++++++++-------
t/t9902-completion.sh | 3 ++
3 files changed, 36 insertions(+), 21 deletions(-)
@@ -113,8 +113,23 @@ sub usage {exit(1);}+subuniq{+my%seen;+grep!$seen{$_}++,@_;+}+subcompletion_helper{-printGit::command('format-patch','--git-completion-helper'),"\n";+my($options)=@_;+my@send_email_opts=map{+"--$_"+}map{+s/(?:[:=][si]|!)$//;+split/\|/,$_;+}keys%$options;+my@format_patch_opts=Git::command('format-patch','--git-completion-helper');+my@options=uniq@send_email_opts,@format_patch_opts;+@options=grep!/--git-completion-helper|--h/,@options;+print"@options\n";exit(0);}
@@ -425,10 +440,11 @@ sub config_regexp {my$key="sendemail.identity";$identity=Git::config(@repo,$key)ifexists$known_config_keys{$key};}-my$rc=GetOptions(-"identity=s"=>\$identity,-"no-identity"=>\$no_identity,+my%identity_options=(+"identity=s"=>\$identity,+"no-identity"=>\$no_identity,);+my$rc=GetOptions(%identity_options);usage()unless$rc;undef$identityif$no_identity;
@@ -444,14 +460,17 @@ sub config_regexp {my$help;my$git_completion_helper;-$rc=GetOptions("h"=>\$help,-"dump-aliases"=>\$dump_aliases);+my%dump_aliases_options=(+"h"=>\$help,+"dump-aliases"=>\$dump_aliases,+);+$rc=GetOptions(%dump_aliases_options);usage()unless$rc;die__("--dump-aliases incompatible with other options\n")if!$helpand$dump_aliasesand@ARGV;-$rc=GetOptions(+my%options=("sender|from=s"=>\$sender,-"in-reply-to=s"=>\$initial_in_reply_to,+"in-reply-to=s"=>\$initial_in_reply_to,"reply-to=s"=>\$reply_to,"subject=s"=>\$initial_subject,"to=s"=>\@getopt_to,
@@ -508,7 +527,8 @@ sub config_regexp {"batch-size=i"=>\$batch_size,"relogin-delay=i"=>\$relogin_delay,"git-completion-helper"=>\$git_completion_helper,-);+);+$rc=GetOptions(%options);# Munge any "either config or getopt, not both" variablesmy@initial_to=@getopt_to?@getopt_to:($no_to?():@config_to);
@@ -516,7 +536,8 @@ sub config_regexp {my@initial_bcc=@getopt_bcc?@getopt_bcc:($no_bcc?():@config_bcc);usage()if$help;-completion_helper()if$git_completion_helper;+my%all_options=(%options,%dump_aliases_options,%identity_options);+completion_helper(\%all_options)if$git_completion_helper;unless($rc){usage();}
this change isn't really needed now that you are not using the usage()
to get the options, and should probably go into an independent patch,
or even better, as part of patch 3 so it is consistent.
quoted hunk
@@ -113,8 +113,23 @@ sub usage { exit(1); }+sub uniq {+ my %seen;+ grep !$seen{$_}++, @_;+}
indentation in this file is a little odd, but will be better if you use the
most common one (tab), instead of your own version here (4 or 2 spaces) and
in the rest of the code.
sub completion_helper {
- print Git::command('format-patch', '--git-completion-helper'), "\n";
+ my ($options) = @_;
+ my @send_email_opts = map {
+ "--$_"
+ } map {
+ s/(?:[:=][si]|!)$//;
+ split /\|/, $_;
+ } keys %$options;
+ my @format_patch_opts = Git::command('format-patch', '--git-completion-helper');
+ my @options = uniq @send_email_opts, @format_patch_opts;
reusing "options" here makes this code more confusing than it needs to be
maybe something like :
From: Carlo Marcelo Arenas Belón <hidden> Date: 2021-10-09 08:31:47
From: Thiago Perrotta <redacted>
The use of <rev-list options> in format-patch is confusing because
it is meant to represent instead a subset of options from format-patch
and a revision range.
Change the documentation from both git send-email and format-patch to
better describe the list of options that are expected.
Signed-off-by: Carlo Marcelo Arenas Belón <redacted>
---
This is an alternative to patch 3 which include most of the suggestions
by Junio and Bagas, while also trying to minimize the change or the need
to link to even more pages (which is why the suggestion[1] to link/use
rev-list options was avoided).
I expect it to be consistent and clear, but might had gotten some of the
terminology wrong, and probably could have better grammar, so hoping for
some feedback to improve on that.
[1] https://lore.kernel.org/git/CABOtWuqXS_kJk2md=kgg-ReaWtKermpUW_Dk_bc0pMXQL+xMeA@mail.gmail.com/T/#m396f2a42936ec9a3191658d4b40d9043dfbc59e2
Documentation/git-format-patch.txt | 20 +++++++++++---------
Documentation/git-send-email.txt | 7 ++++---
2 files changed, 15 insertions(+), 12 deletions(-)
@@ -64,15 +64,17 @@ There are two ways to specify which commits to operate on. to the tip of the current branch that are not in the history that leads to the <since> to be output.-2. Generic <revision range> expression (see "SPECIFYING- REVISIONS" section in linkgit:gitrevisions[7]) means the- commits in the specified range.--The first rule takes precedence in the case of a single <commit>. To-apply the second rule, i.e., format everything since the beginning of-history up until <commit>, use the `--root` option: `git format-patch---root <commit>`. If you want to format only <commit> itself, you-can do this with `git format-patch -1 <commit>`.+2. A Generic <revision range> expression that describes with+ options and revisions a range of commits.++If you give a single <commit> and nothing else, it is taken as the+<since> of the first form. If you want to format everything since the+beginning of history up until <commit>, use the `--root` option:+`git format-patch --root <commit>`. If you want to format only the+<commit> itself, use instead `git format-patch -1 <commit>`.++If you want to affect a set of commits, provide a range (see+"SPECIFYING RANGES" section in linkgit:gitrevisions[7]). By default, each output file is numbered sequentially from 1, and uses the first line of the commit message (massaged for pathname safety) as
@@ -18,8 +19,8 @@ DESCRIPTION Takes the patches given on the command line and emails them out. Patches can be specified as files, directories (which will send all files in the directory), or directly as a revision list. In the-last case, any format accepted by linkgit:git-format-patch[1] can-be passed to git send-email.+last case, a revision in a format accepted by linkgit:git-format-patch[1]+as well as relevant options must be provided to git send-email. The header of the email is configurable via command-line options. If not specified on the command line, the user will be prompted with a ReadLine
On 09/10/21 15.31, Carlo Marcelo Arenas Belón wrote:
-2. Generic <revision range> expression (see "SPECIFYING
- REVISIONS" section in linkgit:gitrevisions[7]) means the
- commits in the specified range.
-
-The first rule takes precedence in the case of a single <commit>. To
-apply the second rule, i.e., format everything since the beginning of
-history up until <commit>, use the `--root` option: `git format-patch
---root <commit>`. If you want to format only <commit> itself, you
-can do this with `git format-patch -1 <commit>`.
+2. A Generic <revision range> expression that describes with
+ options and revisions a range of commits.
+
+If you give a single <commit> and nothing else, it is taken as the
+<since> of the first form. If you want to format everything since the
+beginning of history up until <commit>, use the `--root` option:
+`git format-patch --root <commit>`. If you want to format only the
+<commit> itself, use instead `git format-patch -1 <commit>`.
+
+If you want to affect a set of commits, provide a range (see
+"SPECIFYING RANGES" section in linkgit:gitrevisions[7]).
Supposed that we have following commit graph:
a --- b --- c --- d --- e --- f (main)
\
--- g --- h --- i (mywork, HEAD)
According to your edit, `git format-patch <c>` and `git format-patch
--root <i>` are equivalent, right?
I think for the third case, s/affect/format/.
quoted hunk
@@ -18,8 +19,8 @@ DESCRIPTION Takes the patches given on the command line and emails them out. Patches can be specified as files, directories (which will send all files in the directory), or directly as a revision list. In the-last case, any format accepted by linkgit:git-format-patch[1] can-be passed to git send-email.+last case, a revision in a format accepted by linkgit:git-format-patch[1]+as well as relevant options must be provided to git send-email. The header of the email is configurable via command-line options. If not specified on the command line, the user will be prompted with a ReadLine
Did you mean that in the second form of git send-email, only
format-patch options are accepted?
--
An old man doll... just what I always wanted! - Clara
From: Carlo Arenas <hidden> Date: 2021-10-09 09:33:07
On Sat, Oct 9, 2021 at 1:57 AM Bagas Sanjaya [off-list ref] wrote:
On 09/10/21 15.31, Carlo Marcelo Arenas Belón wrote:
quoted
-2. Generic <revision range> expression (see "SPECIFYING
- REVISIONS" section in linkgit:gitrevisions[7]) means the
- commits in the specified range.
-
-The first rule takes precedence in the case of a single <commit>. To
-apply the second rule, i.e., format everything since the beginning of
-history up until <commit>, use the `--root` option: `git format-patch
---root <commit>`. If you want to format only <commit> itself, you
-can do this with `git format-patch -1 <commit>`.
+2. A Generic <revision range> expression that describes with
+ options and revisions a range of commits.
+
+If you give a single <commit> and nothing else, it is taken as the
+<since> of the first form. If you want to format everything from the
+beginning of history up until <commit>, use the `--root` option:
+`git format-patch --root <commit>`. If you want to format only the
+<commit> itself, use instead `git format-patch -1 <commit>`.
+
+If you want to affect a set of commits, provide a range (see
+"SPECIFYING RANGES" section in linkgit:gitrevisions[7]).
Supposed that we have following commit graph:
a --- b --- c --- d --- e --- f (main)
\
--- g --- h --- i (mywork, HEAD)
According to your edit, `git format-patch <c>` and `git format-patch
--root <i>` are equivalent, right?
I didn't change the definition of --root with my edit, but I guess it
is still confusing.
the beginning of history from your tree is a, c would be a "fork
point" AFAIK, but if you use --root will get 6 patches (a, b, c, g, h,
i)
quoted
@@ -18,8 +19,8 @@ DESCRIPTION Takes the patches given on the command line and emails them out. Patches can be specified as files, directories (which will send all files in the directory), or directly as a revision list. In the-last case, any format accepted by linkgit:git-format-patch[1] can-be passed to git send-email.+last case, a revision in a format accepted by linkgit:git-format-patch[1]+as well as relevant options must be provided to git send-email. The header of the email is configurable via command-line options. If not specified on the command line, the user will be prompted with a ReadLine
Did you mean that in the second form of git send-email, only
format-patch options are accepted?
The synopsis shows "send-email options" are also allowed (as
optional), the mention of "relevant options" here was to indicate
additional options from git-format-patch which make sense (ex: common
diff options, --root, or the options from the range section of
references).
The truth is that you can actually do files, directories and revisions
now, but that is a bug.
Carlo
The synopsis shows "send-email options" are also allowed (as
optional), the mention of "relevant options" here was to indicate
additional options from git-format-patch which make sense (ex: common
diff options, --root, or the options from the range section of
references).
The truth is that you can actually do files, directories and revisions
now, but that is a bug.
Carlo
git send-email behaves similar to ln(1), depending on what options and
arguments that are passed. See my attempted patch [1] for the wording.
[1]:
https://lore.kernel.org/git/20210924121352.42138-1-bagasdotme@gmail.com/
For SYNOPSIS for file/directory and revision range, I suggest:
'git send-email' [<options>] <file|directory>...
'git send-email' [<send-email options>] [<format-patch options>]
<revision range>
where <revision range> must be any forms that are accepted by
git-format-patch(1).
--
An old man doll... just what I always wanted! - Clara
Differences from V6:
2/3: Addresses all of Carlos's comments:
- make indentation consistent (tabs): note that there's a giant diff
for the largest GetOptions now, it adds a bit of noise to the patch
- do not reuse the options variable, for improved readability.
2/3: One extra improvement:
- deduplicate send-email and format-patch common options (for example,
now --from appears only once)
3/3:
- updated git-send-email.perl USAGE to be consistent with
git-send-email(1)
- still haven't touched anything else, waiting on Carlos's RFC patch
ongoing discussion to settle.
Thiago Perrotta (3):
send-email: terminate --git-completion-helper with LF
send-email: programmatically generate bash completions
send-email docs: add format-patch options
Documentation/git-send-email.txt | 6 +-
contrib/completion/git-completion.bash | 11 +-
git-send-email.perl | 156 ++++++++++++++-----------
t/t9902-completion.sh | 3 +
4 files changed, 97 insertions(+), 79 deletions(-)
--
2.33.0
Unlike other Git subcommands, "git send-email" leaves its output an
incomplete line when "--git-completion-helper" is asked. Be consistent
by terminating the message with LF here.
Signed-off-by: Thiago Perrotta <redacted>
---
git-send-email.perl | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
"git send-email --git-completion-helper" only prints "format-patch"
flags. Make it print "send-email" flags as well, generating them
programmatically from the usage.
Extract flags from the three existing `GetOptions`.
Introduce a uniq subroutine, otherwise --cc-cover, --to-cover and other
flags would show up twice. In addition, deduplicate options common to
both send-email and format-patch, like --from.
Remove extraneous flags: --, --h and --git-completion-helper.
Add a completion test for "send-email --validate", a send-email flag.
Signed-off-by: Thiago Perrotta <redacted>
Based-on-patch-by: Ævar Arnfjörð Bjarmason [off-list ref]
---
contrib/completion/git-completion.bash | 11 +-
git-send-email.perl | 153 ++++++++++++++-----------
t/t9902-completion.sh | 3 +
3 files changed, 91 insertions(+), 76 deletions(-)
@@ -113,9 +113,24 @@ sub usage {exit(1);}+subuniq{+my%seen;+grep!$seen{$_}++,@_;+}+subcompletion_helper{-printGit::command('format-patch','--git-completion-helper'),"\n";-exit(0);+my($original_opts)=@_;+my@send_email_opts=map{+"--$_"+}map{+s/(?:[:=][si]|!)$//;+split/\|/;+}keys%$original_opts;+my@format_patch_opts=split(/ /,Git::command('format-patch','--git-completion-helper'));+my@options=(@send_email_opts,@format_patch_opts);+@options=uniq(grep!/^$|^--$|--git-completion-helper|--h/,@options);+print"@options\n";+exit(0);}# most mail servers generate the Date: header, but not all...
@@ -425,10 +440,11 @@ sub config_regexp {my$key="sendemail.identity";$identity=Git::config(@repo,$key)ifexists$known_config_keys{$key};}-my$rc=GetOptions(+my%identity_options=("identity=s"=>\$identity,"no-identity"=>\$no_identity,);+my$rc=GetOptions(%identity_options);usage()unless$rc;undef$identityif$no_identity;
@@ -444,71 +460,75 @@ sub config_regexp {my$help;my$git_completion_helper;-$rc=GetOptions("h"=>\$help,-"dump-aliases"=>\$dump_aliases);+my%dump_aliases_options=(+"h"=>\$help,+"dump-aliases"=>\$dump_aliases,+);+$rc=GetOptions(%dump_aliases_options);usage()unless$rc;die__("--dump-aliases incompatible with other options\n")if!$helpand$dump_aliasesand@ARGV;-$rc=GetOptions(-"sender|from=s"=>\$sender,-"in-reply-to=s"=>\$initial_in_reply_to,-"reply-to=s"=>\$reply_to,-"subject=s"=>\$initial_subject,-"to=s"=>\@getopt_to,-"to-cmd=s"=>\$to_cmd,-"no-to"=>\$no_to,-"cc=s"=>\@getopt_cc,-"no-cc"=>\$no_cc,-"bcc=s"=>\@getopt_bcc,-"no-bcc"=>\$no_bcc,-"chain-reply-to!"=>\$chain_reply_to,-"no-chain-reply-to"=>sub{$chain_reply_to=0},-"sendmail-cmd=s"=>\$sendmail_cmd,-"smtp-server=s"=>\$smtp_server,-"smtp-server-option=s"=>\@smtp_server_options,-"smtp-server-port=s"=>\$smtp_server_port,-"smtp-user=s"=>\$smtp_authuser,-"smtp-pass:s"=>\$smtp_authpass,-"smtp-ssl"=>sub{$smtp_encryption='ssl'},-"smtp-encryption=s"=>\$smtp_encryption,-"smtp-ssl-cert-path=s"=>\$smtp_ssl_cert_path,-"smtp-debug:i"=>\$debug_net_smtp,-"smtp-domain:s"=>\$smtp_domain,-"smtp-auth=s"=>\$smtp_auth,-"no-smtp-auth"=>sub{$smtp_auth='none'},-"annotate!"=>\$annotate,-"no-annotate"=>sub{$annotate=0},-"compose"=>\$compose,-"quiet"=>\$quiet,-"cc-cmd=s"=>\$cc_cmd,-"suppress-from!"=>\$suppress_from,-"no-suppress-from"=>sub{$suppress_from=0},-"suppress-cc=s"=>\@suppress_cc,-"signed-off-cc|signed-off-by-cc!"=>\$signed_off_by_cc,-"no-signed-off-cc|no-signed-off-by-cc"=>sub{$signed_off_by_cc=0},-"cc-cover|cc-cover!"=>\$cover_cc,-"no-cc-cover"=>sub{$cover_cc=0},-"to-cover|to-cover!"=>\$cover_to,-"no-to-cover"=>sub{$cover_to=0},-"confirm=s"=>\$confirm,-"dry-run"=>\$dry_run,-"envelope-sender=s"=>\$envelope_sender,-"thread!"=>\$thread,-"no-thread"=>sub{$thread=0},-"validate!"=>\$validate,-"no-validate"=>sub{$validate=0},-"transfer-encoding=s"=>\$target_xfer_encoding,-"format-patch!"=>\$format_patch,-"no-format-patch"=>sub{$format_patch=0},-"8bit-encoding=s"=>\$auto_8bit_encoding,-"compose-encoding=s"=>\$compose_encoding,-"force"=>\$force,-"xmailer!"=>\$use_xmailer,-"no-xmailer"=>sub{$use_xmailer=0},-"batch-size=i"=>\$batch_size,-"relogin-delay=i"=>\$relogin_delay,-"git-completion-helper"=>\$git_completion_helper,-);+my%options=(+"sender|from=s"=>\$sender,+"in-reply-to=s"=>\$initial_in_reply_to,+"reply-to=s"=>\$reply_to,+"subject=s"=>\$initial_subject,+"to=s"=>\@getopt_to,+"to-cmd=s"=>\$to_cmd,+"no-to"=>\$no_to,+"cc=s"=>\@getopt_cc,+"no-cc"=>\$no_cc,+"bcc=s"=>\@getopt_bcc,+"no-bcc"=>\$no_bcc,+"chain-reply-to!"=>\$chain_reply_to,+"no-chain-reply-to"=>sub{$chain_reply_to=0},+"sendmail-cmd=s"=>\$sendmail_cmd,+"smtp-server=s"=>\$smtp_server,+"smtp-server-option=s"=>\@smtp_server_options,+"smtp-server-port=s"=>\$smtp_server_port,+"smtp-user=s"=>\$smtp_authuser,+"smtp-pass:s"=>\$smtp_authpass,+"smtp-ssl"=>sub{$smtp_encryption='ssl'},+"smtp-encryption=s"=>\$smtp_encryption,+"smtp-ssl-cert-path=s"=>\$smtp_ssl_cert_path,+"smtp-debug:i"=>\$debug_net_smtp,+"smtp-domain:s"=>\$smtp_domain,+"smtp-auth=s"=>\$smtp_auth,+"no-smtp-auth"=>sub{$smtp_auth='none'},+"annotate!"=>\$annotate,+"no-annotate"=>sub{$annotate=0},+"compose"=>\$compose,+"quiet"=>\$quiet,+"cc-cmd=s"=>\$cc_cmd,+"suppress-from!"=>\$suppress_from,+"no-suppress-from"=>sub{$suppress_from=0},+"suppress-cc=s"=>\@suppress_cc,+"signed-off-cc|signed-off-by-cc!"=>\$signed_off_by_cc,+"no-signed-off-cc|no-signed-off-by-cc"=>sub{$signed_off_by_cc=0},+"cc-cover|cc-cover!"=>\$cover_cc,+"no-cc-cover"=>sub{$cover_cc=0},+"to-cover|to-cover!"=>\$cover_to,+"no-to-cover"=>sub{$cover_to=0},+"confirm=s"=>\$confirm,+"dry-run"=>\$dry_run,+"envelope-sender=s"=>\$envelope_sender,+"thread!"=>\$thread,+"no-thread"=>sub{$thread=0},+"validate!"=>\$validate,+"no-validate"=>sub{$validate=0},+"transfer-encoding=s"=>\$target_xfer_encoding,+"format-patch!"=>\$format_patch,+"no-format-patch"=>sub{$format_patch=0},+"8bit-encoding=s"=>\$auto_8bit_encoding,+"compose-encoding=s"=>\$compose_encoding,+"force"=>\$force,+"xmailer!"=>\$use_xmailer,+"no-xmailer"=>sub{$use_xmailer=0},+"batch-size=i"=>\$batch_size,+"relogin-delay=i"=>\$relogin_delay,+"git-completion-helper"=>\$git_completion_helper,+);+$rc=GetOptions(%options);# Munge any "either config or getopt, not both" variablesmy@initial_to=@getopt_to?@getopt_to:($no_to?():@config_to);
@@ -516,7 +536,8 @@ sub config_regexp {my@initial_bcc=@getopt_bcc?@getopt_bcc:($no_bcc?():@config_bcc);usage()if$help;-completion_helper()if$git_completion_helper;+my%all_options=(%options,%dump_aliases_options,%identity_options);+completion_helper(\%all_options)if$git_completion_helper;unless($rc){usage();}
git-send-email(1) does not mention that "git format-patch" options are
accepted. Augment SYNOPSIS and DESCRIPTION to mention it.
Update git-send-email.perl USAGE to be consistent with
git-send-email(1).
Signed-off-by: Thiago Perrotta <redacted>
---
Documentation/git-send-email.txt | 6 ++++--
git-send-email.perl | 3 ++-
2 files changed, 6 insertions(+), 3 deletions(-)
@@ -9,7 +9,8 @@ git-send-email - Send a collection of patches as emails SYNOPSIS -------- [verse]-'git send-email' [<options>] <file|directory|rev-list options>...+'git send-email' [<options>] <file|directory>...+'git send-email' [<options>] <format-patch options> 'git send-email' --dump-aliases
@@ -19,7 +20,8 @@ Takes the patches given on the command line and emails them out. Patches can be specified as files, directories (which will send all files in the directory), or directly as a revision list. In the last case, any format accepted by linkgit:git-format-patch[1] can-be passed to git send-email.+be passed to git send-email, as well as options understood by+linkgit:git-format-patch[1]. The header of the email is configurable via command-line options. If not specified on the command line, the user will be prompted with a ReadLine
Differences from V6:
2/3: Addresses all of Carlos's comments:
- make indentation consistent (tabs): note that there's a giant diff
for the largest GetOptions now, it adds a bit of noise to the patch
I took Carlo's suggestion to mean to indent your uniq function, not to
re-indent a bunch of existing code while at it...
- do not reuse the options variable, for improved readability.
...I think that re-indentation is better left alone for the patch
readability.
Anyway, sorry about not looking at this sooner after my off-the-cuff
[1]; I think this looks mostly good-ish, but there's a few broken things
here:
First, in your 1/3 you're adding a \n, but in 2/3 we end up with \n\n. I
think you can just skip 1/3, maybe mention "how it also has a "\n" in
the commit message.
I.e. you start implicitly picking up the newline because you changed
from a "print" to a "split", and latter imposes Perl's scalar context on
its argument, but the former doesn't. That's a combination of some Perl
trivia and our own Git.pm being overly clever about wantarray(), but
there you go.
More importantly in [1] I meant that last paragraph as a "and as an
excercise for the reader..".
I.e. we should not simply strip the trailing "=" etc., we need to parse
those out of the Perl GetOptions arguments, and come up with mapping to
what we do in parse-options.c. I think that's basically adding a "=" at
the end of all options that end with "=s", ":i", "=d", ":s" etc.
You then strip out "--" arguments from the combined list, but isn't this
something we do need to emit? I.e. it's used as syntax by the bash
completion isn't it? (I just skimmed the relevant C code in
parse-options.c).
$ git clone --git-completion-helper | tr ' ' '\n' | grep -C2 -- ^--$
--hardlinks
--tags
--
--no-verbose
--no-quiet
For --no-foo arguments we emit both a --foo and --no-foo in it, that
sort of (maybe entirely) works in your version because some/all (I
haven't checked all) options have corresponding "foo" arguments for
"no-foo", so maybe it sort of works out, but does the ordering
before/after the "--", and that we strip out the "--" but e.g. "git
clone" will emit it?
We then don't want to emit "-h", but you strip out "--h", first we
mapped "h" to "--h" in the loop above, so we should do that there. But
better yet we have a "%dump_aliases_options" already, maybe it +
"git-completion-helper" can be moved to another "%not_for_completion"
hash or something.
The map/map/keys loop also will silently do something odd if it starts
getting input data it didn't expect, i.e. it would be more reliable as a
regex check, and then die() if we start getting somethnig we don't
expect, i.e. if someone adds an option not covered by our regex.
That it's a map/map/keys is just some off-the-cuff Perl hacking on my
part, I think for validation etc. it's usually better to just turn it
into a plain old boring for-loop.
1. https://lore.kernel.org/git/87bl4h3fgv.fsf@evledraar.gmail.com/
"git send-email --git-completion-helper" only prints "format-patch"
flags. Make it print "send-email" flags as well, extracting them
programmatically from its three existing "GetOptions".
Introduce a "uniq" subroutine, otherwise --cc-cover, --to-cover and
other flags would show up twice. In addition, deduplicate flags common
to both "send-email" and "format-patch", like --from.
Remove extraneous flags: --, --h and --git-completion-helper.
Add a completion test for "send-email --validate", a send-email flag.
Signed-off-by: Thiago Perrotta <redacted>
Based-on-patch-by: Ævar Arnfjörð Bjarmason [off-list ref]
---
...I think that re-indentation is better left alone for the patch
readability.
Reverted the `GetOptions` indentation. Noise is now gone :-)
First, in your 1/3 you're adding a \n, but in 2/3 we end up with \n\n. I
think you can just skip 1/3, maybe mention "how it also has a "\n" in
the commit message.
I don't quite see how this would fit into the commit message. A comment in the
code seems to fit better to account for this detail. That's what I did, but if
you still disagree, please elaborate where in the commit message this sentence
should be added.
You then strip out "--" arguments from the combined list, but isn't this
something we do need to emit? I.e. it's used as syntax by the bash
completion isn't it? (I just skimmed the relevant C code in
parse-options.c).
I interpreted that standalone `--` as an extraneous / useless token. If it's
there intentionally, then I am reverting my stripping it. At the end of the day
whether to include it or not is a small detail, but FYI, when I do:
git clone -<TAB>
in bash, nothing happens. I would have expected it to be expanded to "--"
because of the explicit "--", but it doesn't. Therefore my conclusion is that
"--" in the output of "--git-completion-helper" is useless. Am I missing
something?
Finally, as per your comments about "=": OK, I see what you mean. For the record
I had produced a quick-and-dirty version earlier (on top of V6), but didn't
include it in V7 because I didn't deem it polished enough, but I'll include it
here for reference. I'll take your new comments to see if I can produce a better
polished version before I send V8:
@@ -123,12 +123,13 @@ sub completion_helper {my@send_email_opts=map{"--$_"}map{-s/(?:[:=][si]|!)$//;+s/!$//;+s/[:=][si]$/=/;split/\|/,$_;}keys%$options;-my@format_patch_opts=Git::command('format-patch','--git-completion-helper');-my@options=uniq@send_email_opts,@format_patch_opts;-@options=grep!/--git-completion-helper|--h/,@options;+my@format_patch_opts=split(/ /,Git::command('format-patch','--git-completion-helper'));+my@options=(@send_email_opts,@format_patch_opts);+@options=uniq(grep!/^$|^--$|--git-completion-helper|--h/,@options);print"@options\n";exit(0);}
That it's a map/map/keys is just some off-the-cuff Perl hacking on my
part, I think for validation etc. it's usually better to just turn it
into a plain old boring for-loop.
Oh, okay, I didn't find it hacky at all, the `map` seemed quite elegant and
functional to me, but then again I am a perl newbie :-P.
Differences from V7 (so far):
As per Ævar's comments:
- completely drop 1/3 since we'now using split instead of print to incorporate
format-patch options. Side effect: then-extraneous '\n' is now gone.
- revert indentation (tabs) on GetOptions to eliminate diff noise (as per Ævar's
interpretation of original Carlos's comment)
- re-add "--" to the output of "git send-email --git-completion-helper"
contrib/completion/git-completion.bash | 11 +------
git-send-email.perl | 40 ++++++++++++++++++++------
t/t9902-completion.sh | 3 ++
3 files changed, 35 insertions(+), 19 deletions(-)
@@ -113,9 +113,25 @@ sub usage {exit(1);}+subuniq{+my%seen;+grep!$seen{$_}++,@_;+}+subcompletion_helper{-printGit::command('format-patch','--git-completion-helper');-exit(0);+my($original_opts)=@_;+my@send_email_opts=map{+"--$_"+}map{+s/(?:[:=][si]|!)$//;+split/\|/;+}keys%$original_opts;+my@format_patch_opts=split(/ /,Git::command('format-patch','--git-completion-helper'));+my@options=(@send_email_opts,@format_patch_opts);+@options=uniq(grep!/^$|--git-completion-helper|--h/,@options);+# There's an implicit '\n' here already, no need to add an explicit one.+print"@options";+exit(0);}# most mail servers generate the Date: header, but not all...
@@ -425,10 +441,11 @@ sub config_regexp {my$key="sendemail.identity";$identity=Git::config(@repo,$key)ifexists$known_config_keys{$key};}-my$rc=GetOptions(+my%identity_options=("identity=s"=>\$identity,"no-identity"=>\$no_identity,);+my$rc=GetOptions(%identity_options);usage()unless$rc;undef$identityif$no_identity;
@@ -444,14 +461,17 @@ sub config_regexp {my$help;my$git_completion_helper;-$rc=GetOptions("h"=>\$help,-"dump-aliases"=>\$dump_aliases);+my%dump_aliases_options=(+"h"=>\$help,+"dump-aliases"=>\$dump_aliases,+);+$rc=GetOptions(%dump_aliases_options);usage()unless$rc;die__("--dump-aliases incompatible with other options\n")if!$helpand$dump_aliasesand@ARGV;-$rc=GetOptions(+my%options=("sender|from=s"=>\$sender,-"in-reply-to=s"=>\$initial_in_reply_to,+"in-reply-to=s"=>\$initial_in_reply_to,"reply-to=s"=>\$reply_to,"subject=s"=>\$initial_subject,"to=s"=>\@getopt_to,
@@ -508,7 +528,8 @@ sub config_regexp {"batch-size=i"=>\$batch_size,"relogin-delay=i"=>\$relogin_delay,"git-completion-helper"=>\$git_completion_helper,-);+);+$rc=GetOptions(%options);# Munge any "either config or getopt, not both" variablesmy@initial_to=@getopt_to?@getopt_to:($no_to?():@config_to);
@@ -516,7 +537,8 @@ sub config_regexp {my@initial_bcc=@getopt_bcc?@getopt_bcc:($no_bcc?():@config_bcc);usage()if$help;-completion_helper()if$git_completion_helper;+my%all_options=(%options,%dump_aliases_options,%identity_options);+completion_helper(\%all_options)if$git_completion_helper;unless($rc){usage();}
...I think that re-indentation is better left alone for the patch
readability.
Reverted the `GetOptions` indentation. Noise is now gone :-)
First, in your 1/3 you're adding a \n, but in 2/3 we end up with \n\n. I
think you can just skip 1/3, maybe mention "how it also has a "\n" in
the commit message.
I don't quite see how this would fit into the commit message. A comment in the
code seems to fit better to account for this detail. That's what I did, but if
you still disagree, please elaborate where in the commit message this sentence
should be added.
You then strip out "--" arguments from the combined list, but isn't this
something we do need to emit? I.e. it's used as syntax by the bash
completion isn't it? (I just skimmed the relevant C code in
parse-options.c).
I interpreted that standalone `--` as an extraneous / useless token. If it's
there intentionally, then I am reverting my stripping of it. At the end of the
day whether to include it or not is a small detail, but FYI, when I do:
$ git clone -<TAB>
in bash, nothing happens. I would have expected it to be expanded to "--"
because of the explicit "--", but it doesn't. Therefore my conclusion is that
"--" in the output of "--git-completion-helper" is useless. Am I missing
something? What would be the function of the standalone "--" then?
From my local testing, whether the options are sorted or not, whether
they are repeated or not, whether they follow a specific order with
respect to "--" or not, all of those details seem not to matter. Bash
completion seems to handle all of those cases just fine interactively.
In fact, here's another example:
$ git init --git-completion-helper | tr ' ' '\n' | grep -C1 '^--$'
--no-template
--
--no-bare
...there are --no-* options both _before_ and _after_ the --. I really
cannot see the point of the -- in the output, it seems to be just noise.
I readded -- to the output anyway since you requested it, but if it
needs to follow a certain spec w.r.t. ordering, we should have tests for
it. This specific part (the -- and the --no- order thing) of the commit
is something I am not keen to doing though, at least not in this patch
series; sorry, it already goes far beyond the scope of my original
intent. Anything else you ask for that is inline with the original
intent (like generating options programatically instead of hard-coding
them) I am fine with though, and in fact I believe I have addressed all
comments so far, if there's anything else I may have missed let me know.
I.e. we should not simply strip the trailing "=" etc., we need to parse
those out of the Perl GetOptions arguments, and come up with mapping to
what we do in parse-options.c. I think that's basically adding a "=" at
the end of all options that end with "=s", ":i", "=d", ":s" etc.
OK, I see. This is a valid point, I updated the code to reflect this desired
behavior. PTAL. If needed we could make it more DRY, but as it is now it
seems quite readable.
We then don't want to emit "-h", but you strip out "--h", first we
mapped "h" to "--h" in the loop above, so we should do that there. But
better yet we have a "%dump_aliases_options" already, maybe it +
"git-completion-helper" can be moved to another "%not_for_completion"
hash or something.
OK, done. Introduced a "not_for_completion" hash. Doesn't seem to make a
huge difference compared to just using `grep` though.
Differences from V7:
As per Ævar's comments:
- completely drop 1/3 since we'now using split instead of print to incorporate
format-patch options. Side effect: then-extraneous '\n' is now gone.
- revert indentation (tabs) on GetOptions to eliminate diff noise (as per Ævar's
interpretation of original Carlos's comment)
- re-add "--" to the output of "git send-email --git-completion-helper"
- introduce a "not_for_completion" hash to remove undesired options from the
output
- add trailing = to send-email options that expect a string or an integer
argument, consistently with format-patch options
Thiago Perrotta (2):
send-email: programmatically generate bash completions
send-email docs: add format-patch options
Documentation/git-send-email.txt | 6 ++-
contrib/completion/git-completion.bash | 11 +----
git-send-email.perl | 56 +++++++++++++++++++++-----
t/t9902-completion.sh | 3 ++
4 files changed, 54 insertions(+), 22 deletions(-)
--
2.33.1
"git send-email --git-completion-helper" only prints "format-patch"
flags. Make it print "send-email" flags as well, extracting them
programmatically from its three existing "GetOptions".
Introduce a "uniq" subroutine, otherwise --cc-cover, --to-cover and
other flags would show up twice. In addition, deduplicate flags common
to both "send-email" and "format-patch", like --from.
Remove extraneous flags: --h and --git-completion-helper.
Add trailing "=" to options that expect an argument, inline with
the format-patch implementation.
Add a completion test for "send-email --validate", a send-email flag.
Signed-off-by: Thiago Perrotta <redacted>
Based-on-patch-by: Ævar Arnfjörð Bjarmason [off-list ref]
---
contrib/completion/git-completion.bash | 11 +-----
git-send-email.perl | 53 +++++++++++++++++++++-----
t/t9902-completion.sh | 3 ++
3 files changed, 48 insertions(+), 19 deletions(-)
@@ -113,9 +113,38 @@ sub usage {exit(1);}+subuniq{+my%seen;+grep!$seen{$_}++,@_;+}+subcompletion_helper{-printGit::command('format-patch','--git-completion-helper');-exit(0);+my($original_opts)=@_;+my%not_for_completion=(+"git-completion-helper"=>undef,+"h"=>undef,+);+my@send_email_opts=();++foreachmy$key(keys%$original_opts){+unless(exists$not_for_completion{$key}){+$key=~s/!$//;++if($key=~ /[:=][si]$/){+$key=~s/[:=][si]$//;+push(@send_email_opts,"--$_=")foreach(split(/\|/,$key));+}else{+push(@send_email_opts,"--$_")foreach(split(/\|/,$key));+}+}+}++my@format_patch_opts=split(/ /,Git::command('format-patch','--git-completion-helper'));+my@opts=(@send_email_opts,@format_patch_opts);+@opts=uniq(grep!/^$/,@opts);+# There's an implicit '\n' here already, no need to add an explicit one.+print"@opts";+exit(0);}# most mail servers generate the Date: header, but not all...
@@ -425,10 +454,11 @@ sub config_regexp {my$key="sendemail.identity";$identity=Git::config(@repo,$key)ifexists$known_config_keys{$key};}-my$rc=GetOptions(+my%identity_options=("identity=s"=>\$identity,"no-identity"=>\$no_identity,);+my$rc=GetOptions(%identity_options);usage()unless$rc;undef$identityif$no_identity;
@@ -444,14 +474,17 @@ sub config_regexp {my$help;my$git_completion_helper;-$rc=GetOptions("h"=>\$help,-"dump-aliases"=>\$dump_aliases);+my%dump_aliases_options=(+"h"=>\$help,+"dump-aliases"=>\$dump_aliases,+);+$rc=GetOptions(%dump_aliases_options);usage()unless$rc;die__("--dump-aliases incompatible with other options\n")if!$helpand$dump_aliasesand@ARGV;-$rc=GetOptions(+my%options=("sender|from=s"=>\$sender,-"in-reply-to=s"=>\$initial_in_reply_to,+"in-reply-to=s"=>\$initial_in_reply_to,"reply-to=s"=>\$reply_to,"subject=s"=>\$initial_subject,"to=s"=>\@getopt_to,
@@ -508,7 +541,8 @@ sub config_regexp {"batch-size=i"=>\$batch_size,"relogin-delay=i"=>\$relogin_delay,"git-completion-helper"=>\$git_completion_helper,-);+);+$rc=GetOptions(%options);# Munge any "either config or getopt, not both" variablesmy@initial_to=@getopt_to?@getopt_to:($no_to?():@config_to);
@@ -516,7 +550,8 @@ sub config_regexp {my@initial_bcc=@getopt_bcc?@getopt_bcc:($no_bcc?():@config_bcc);usage()if$help;-completion_helper()if$git_completion_helper;+my%all_options=(%options,%dump_aliases_options,%identity_options);+completion_helper(\%all_options)if$git_completion_helper;unless($rc){usage();}
git-send-email(1) does not mention that "git format-patch" options are
accepted. Augment SYNOPSIS and DESCRIPTION to mention it.
Update git-send-email.perl USAGE to be consistent with
git-send-email(1).
Signed-off-by: Thiago Perrotta <redacted>
---
Documentation/git-send-email.txt | 6 ++++--
git-send-email.perl | 3 ++-
2 files changed, 6 insertions(+), 3 deletions(-)
@@ -9,7 +9,8 @@ git-send-email - Send a collection of patches as emails SYNOPSIS -------- [verse]-'git send-email' [<options>] <file|directory|rev-list options>...+'git send-email' [<options>] <file|directory>...+'git send-email' [<options>] <format-patch options> 'git send-email' --dump-aliases
@@ -19,7 +20,8 @@ Takes the patches given on the command line and emails them out. Patches can be specified as files, directories (which will send all files in the directory), or directly as a revision list. In the last case, any format accepted by linkgit:git-format-patch[1] can-be passed to git send-email.+be passed to git send-email, as well as options understood by+linkgit:git-format-patch[1]. The header of the email is configurable via command-line options. If not specified on the command line, the user will be prompted with a ReadLine
...I think that re-indentation is better left alone for the patch
readability.
Reverted the `GetOptions` indentation. Noise is now gone :-)
Thanks.
quoted
First, in your 1/3 you're adding a \n, but in 2/3 we end up with \n\n. I
think you can just skip 1/3, maybe mention "how it also has a "\n" in
the commit message.
I don't quite see how this would fit into the commit message. A comment in the
code seems to fit better to account for this detail. That's what I did, but if
you still disagree, please elaborate where in the commit message this sentence
should be added.
Makes sense I think, will take a look.
quoted
You then strip out "--" arguments from the combined list, but isn't this
something we do need to emit? I.e. it's used as syntax by the bash
completion isn't it? (I just skimmed the relevant C code in
parse-options.c).
I interpreted that standalone `--` as an extraneous / useless token. If it's
there intentionally, then I am reverting my stripping of it. At the end of the
day whether to include it or not is a small detail, but FYI, when I do:
$ git clone -<TAB>
in bash, nothing happens. I would have expected it to be expanded to "--"
because of the explicit "--", but it doesn't. Therefore my conclusion is that
"--" in the output of "--git-completion-helper" is useless. Am I missing
something? What would be the function of the standalone "--" then?
From my local testing, whether the options are sorted or not, whether
they are repeated or not, whether they follow a specific order with
respect to "--" or not, all of those details seem not to matter. Bash
completion seems to handle all of those cases just fine interactively.
Digging a bit more: It's for folding away options that are negated, not
for completing "-<TAB>". See the examples at b221b5ab9b9 (completion:
collapse extra --no-.. options, 2018-06-06).
In fact, here's another example:
$ git init --git-completion-helper | tr ' ' '\n' | grep -C1 '^--$'
--no-template
--
--no-bare
...there are --no-* options both _before_ and _after_ the --. I really
cannot see the point of the -- in the output, it seems to be just noise.
Right, because some --no-whatever we define because we've got a
--whatever and it's boolean, but for others we've got a --no-whatever as
the primary, as in th case of --no-template..
I readded -- to the output anyway since you requested it, but if it
needs to follow a certain spec w.r.t. ordering, we should have tests for
it. This specific part (the -- and the --no- order thing) of the commit
is something I am not keen to doing though, at least not in this patch
series; sorry, it already goes far beyond the scope of my original
intent. Anything else you ask for that is inline with the original
intent (like generating options programatically instead of hard-coding
them) I am fine with though, and in fact I believe I have addressed all
comments so far, if there's anything else I may have missed let me know.
Yeah sorry about the confusion so far, it's also been a voyage of
discovery for me :)
This time around I tested with:
On Tue, Oct 26 2021, Ævar Arnfjörð Bjarmason wrote:
quoted hunk
On Mon, Oct 25 2021, Thiago Perrotta wrote:
quoted
quoted
...I think that re-indentation is better left alone for the patch
readability.
Reverted the `GetOptions` indentation. Noise is now gone :-)
Thanks.
quoted
quoted
First, in your 1/3 you're adding a \n, but in 2/3 we end up with \n\n. I
think you can just skip 1/3, maybe mention "how it also has a "\n" in
the commit message.
I don't quite see how this would fit into the commit message. A comment in the
code seems to fit better to account for this detail. That's what I did, but if
you still disagree, please elaborate where in the commit message this sentence
should be added.
Makes sense I think, will take a look.
quoted
quoted
You then strip out "--" arguments from the combined list, but isn't this
something we do need to emit? I.e. it's used as syntax by the bash
completion isn't it? (I just skimmed the relevant C code in
parse-options.c).
I interpreted that standalone `--` as an extraneous / useless token. If it's
there intentionally, then I am reverting my stripping of it. At the end of the
day whether to include it or not is a small detail, but FYI, when I do:
$ git clone -<TAB>
in bash, nothing happens. I would have expected it to be expanded to "--"
because of the explicit "--", but it doesn't. Therefore my conclusion is that
"--" in the output of "--git-completion-helper" is useless. Am I missing
something? What would be the function of the standalone "--" then?
From my local testing, whether the options are sorted or not, whether
they are repeated or not, whether they follow a specific order with
respect to "--" or not, all of those details seem not to matter. Bash
completion seems to handle all of those cases just fine interactively.
Digging a bit more: It's for folding away options that are negated, not
for completing "-<TAB>". See the examples at b221b5ab9b9 (completion:
collapse extra --no-.. options, 2018-06-06).
quoted
In fact, here's another example:
$ git init --git-completion-helper | tr ' ' '\n' | grep -C1 '^--$'
--no-template
--
--no-bare
...there are --no-* options both _before_ and _after_ the --. I really
cannot see the point of the -- in the output, it seems to be just noise.
Right, because some --no-whatever we define because we've got a
--whatever and it's boolean, but for others we've got a --no-whatever as
the primary, as in th case of --no-template..
quoted
I readded -- to the output anyway since you requested it, but if it
needs to follow a certain spec w.r.t. ordering, we should have tests for
it. This specific part (the -- and the --no- order thing) of the commit
is something I am not keen to doing though, at least not in this patch
series; sorry, it already goes far beyond the scope of my original
intent. Anything else you ask for that is inline with the original
intent (like generating options programatically instead of hard-coding
them) I am fine with though, and in fact I believe I have addressed all
comments so far, if there's anything else I may have missed let me know.
Yeah sorry about the confusion so far, it's also been a voyage of
discovery for me :)
This time around I tested with:
Which will fail a test in t/t9902-completion.sh showing this specific
behavior.
FWIW I came up with the below on top while testing this. I think your
patch series is fine and we should just take it as-is.
This edge case of "--no" behavior isn't something we support in any
sensible way already, so it can just be left for later.
But since I think I gave you some bad advice and WIP code (sorry!) early
on I think this is closer to a 1=1 mapping to parse-options.c behavior,
i.e. we split up "no" and "non-no" options, put them on either side of
the "--", but are careful to put /some/ "--no" options on the RHS.
This has at least one bug: A few options have e.g. --no-to= and --no-to,
so both "=" and "" forms.
There's also some unrelated fixes there, e.g. the existing
--dump-aliases behavior was kind of dumb for what it was trying to do,
and I changed it since it got in the way of hacking this up.
@@ -120,32 +120,87 @@ sub uniq {}subcompletion_helper{-my($original_opts)=@_;-my%not_for_completion=(-"git-completion-helper"=>undef,-"h"=>undef,+my($options,$no_opt)=@_;++my%fp_opt_types;+my(@fp_pos_opt,@fp_neg_opt);+my$saw_dashdash=0;+my$fp_gch=Git::command('format-patch','--git-completion-helper');+formy$opt(split' ',$fp_gch){+if($opteq'--'){+$saw_dashdash=1;+next;+}+$opt=~s/^--//;+my$str=$opt=~s/=$//;+if($saw_dashdash){+push@fp_neg_opt=>$opt;+}else{+push@fp_pos_opt=>$opt;+}+$fp_opt_types{$opt}=$str?'=':'';+}++## Parse options according to Getopt::Long specifications. See+## "perldoc Getopt::Long".+my%se_opt_types;+my(@se_pos_opt,@se_neg_opt);+formy$item([$options,\@se_pos_opt],[$no_opt,\@se_neg_opt]){+my($orig,$new)=@$item;+@$new=map{+my$type;+$type=''if!$type&&s/\!$//;+$type='='if!$type&&s/[=:][sid]$//;+$type=''if!$type;# the default+# We don't handle all possible Getopt::Long argument specifications+die"BUG: unknown argument specification for $_"if/[=:+]/;+$se_opt_types{$_}=$typeforsplit/\|/;+split/\|/;+}keys%$orig;+}++## Sanity check that format-patch options and send-email+## options don't clash. We have existing conflicts, but should+## not be adding more.+my%conflicting;+my@conflicting=qw(+cc+from+in-reply-to+no-cc+no-thread+no-to+quiet+thread+to);-my@send_email_opts=();--foreachmy$key(keys%$original_opts){-unless(exists$not_for_completion{$key}){-$key=~s/!$//;+@conflicting{@conflicting}=();+formy$opt(@fp_neg_opt,@fp_pos_opt){+nextunlessexists$se_opt_types{$opt};+nextifexists$conflicting{$opt};+warn"BUG: have a format-patch option '$opt' clashing with send-email";+}-if($key=~ /[:=][si]$/){-$key=~s/[:=][si]$//;-push(@send_email_opts,"--$_=")foreach(split(/\|/,$key));-}else{-push(@send_email_opts,"--$_")foreach(split(/\|/,$key));-}-}+## Sanity check that for a --no-foo we have a positive --foo+my@se_bool_neg_opt=grep{/^no-/and$se_opt_types{$_}eq''}keys%se_opt_types;+formy$opt(sort@se_bool_neg_opt){+my$pos=$opt;$pos=~s/^no-//;+nextifexists$se_opt_types{$pos};+die"BUG: Should have a '$pos' corresponding to '$opt'";}-my@format_patch_opts=split(/ /,Git::command('format-patch','--git-completion-helper'));-my@opts=(@send_email_opts,@format_patch_opts);-@opts=uniq(grep!/^$/,@opts);-# There's an implicit '\n' here already, no need to add an explicit one.-print"@opts";-exit(0);+my@pos_opts=sort(+map({"--$_$fp_opt_types{$_}"}@fp_pos_opt),+map({"--$_$se_opt_types{$_}"}@se_pos_opt),+);+my@neg_opts=sort(+map({"--$_$fp_opt_types{$_}"}@fp_neg_opt),+map({"--$_$se_opt_types{$_}"}@se_neg_opt),+);+my@all_opts=(uniq(@pos_opts),"--",uniq(@neg_opts));++print"@all_opts\n";+exit;}# most mail servers generate the Date: header, but not all...
@@ -470,20 +525,14 @@ sub config_regexp {read_config(\%known_config_keys,\%configured,"sendemail");}-# Begin by accumulating all the variables (defined above), that we will end up-# needing, first, from the command line:--my$help;-my$git_completion_helper;-my%dump_aliases_options=(-"h"=>\$help,-"dump-aliases"=>\$dump_aliases,+my$help=0;+my$git_completion_helper=0;+my%options_no_completion=(+"h"=>\$help,+"git-completion-helper"=>\$git_completion_helper,);-$rc=GetOptions(%dump_aliases_options);-usage()unless$rc;-die__("--dump-aliases incompatible with other options\n")-if!$helpand$dump_aliasesand@ARGV;my%options=(+"dump-aliases"=>\$dump_aliases,"sender|from=s"=>\$sender,"in-reply-to=s"=>\$initial_in_reply_to,"reply-to=s"=>\$reply_to,
@@ -528,9 +577,7 @@ sub config_regexp {"dry-run"=>\$dry_run,"envelope-sender=s"=>\$envelope_sender,"thread!"=>\$thread,-"no-thread"=>sub{$thread=0},"validate!"=>\$validate,-"no-validate"=>sub{$validate=0},"transfer-encoding=s"=>\$target_xfer_encoding,"format-patch!"=>\$format_patch,"no-format-patch"=>sub{$format_patch=0},
@@ -538,12 +585,23 @@ sub config_regexp {"compose-encoding=s"=>\$compose_encoding,"force"=>\$force,"xmailer!"=>\$use_xmailer,-"no-xmailer"=>sub{$use_xmailer=0},"batch-size=i"=>\$batch_size,"relogin-delay=i"=>\$relogin_delay,-"git-completion-helper"=>\$git_completion_helper,);-$rc=GetOptions(%options);+# --no-* options that disable a default that's otherwise enabled by+# default. Matters for --git-completion-helper.+my%no_options=(+"no-thread"=>sub{$thread=0},+"no-validate"=>sub{$validate=0},+"no-xmailer"=>sub{$use_xmailer=0},+);+my@ORIG_ARGV=@ARGV;+$rc=GetOptions(%options_no_completion,%options);+usage()unless$rc;++# Option compatibility+die__("--dump-aliases incompatible with other options\n")+if$dump_aliasesand@ORIG_ARGV>1;# Munge any "either config or getopt, not both" variablesmy@initial_to=@getopt_to?@getopt_to:($no_to?():@config_to);
@@ -551,11 +609,7 @@ sub config_regexp {my@initial_bcc=@getopt_bcc?@getopt_bcc:($no_bcc?():@config_bcc);usage()if$help;-my%all_options=(%options,%dump_aliases_options,%identity_options);-completion_helper(\%all_options)if$git_completion_helper;-unless($rc){-usage();-}+completion_helper({%options,%identity_options},\%no_options)if$git_completion_helper;if($forbid_sendmail_variables&&grep{/^sendmail/s}keys%known_config_keys){die__("fatal: found configuration options for 'sendmail'\n".