| Message ID | 20231124103932.31ca7688@pc09 (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Fix git-send-email.perl w.r.t. recent Getopt::Long update | expand |
On Fri, Nov 24, 2023 at 10:39:32AM +0100, H.Merijn Brand wrote:
> Patch attached
Do not send patches as attachments; send them inline instead. See
Documentation/SubmittingPatches for more info (hint: send patches
with git-send-email(1)).
Thanks.
On Sat, 25 Nov 2023 09:22:29 +0700, Bagas Sanjaya <bagasdotme@gmail.com> wrote: > On Fri, Nov 24, 2023 at 10:39:32AM +0100, H.Merijn Brand wrote: > > Patch attached > > Do not send patches as attachments; send them inline instead. See > Documentation/SubmittingPatches for more info (hint: send patches > with git-send-email(1)). As I am used to PR's by now on all OSS projects I am involved in, or use git commits or merges directly on the repo, I *never* use format-patch and/or send-email. These docs - yes I read them - do not offer a concise cut-n-paste example for people like me. In order to have my relative simple patch submitted (I already had the PR ready, but that came with a huge warning that PR's are not accepted) I did it the way I did it. Now I need to read and learn two new commands> I don't think that is very user-friendly, but that might be just me. Ironically, this patch is about the mail part of git. I suggest adding a small example like # Create the patch basics $ git format-patch --cover-letter -M origin/master -o outgoing # Fix the subject $ $VISUAL outgoing/0000-cover-letter.patch # Send the mail $ git send-email --to=git@vger.kernel.org outgoing/* > Thanks.
On 2023-11-25 10:45, H.Merijn Brand wrote: > As I am used to PR's by now on all OSS projects I am involved in, or > use git commits or merges directly on the repo, I *never* use > format-patch and/or send-email. Actually, using email to send patches is quite neat, IMHO, once you get hold of it. > These docs - yes I read them - do not offer a concise cut-n-paste > example for people like me. In order to have my relative simple patch > submitted (I already had the PR ready, but that came with a huge > warning that PR's are not accepted) I did it the way I did it. Now I > need to read and learn two new commands> I don't think that is very > user-friendly, but that might be just me. > > Ironically, this patch is about the mail part of git. > > I suggest adding a small example like > > # Create the patch basics > $ git format-patch --cover-letter -M origin/master -o outgoing > # Fix the subject > $ $VISUAL outgoing/0000-cover-letter.patch > # Send the mail > $ git send-email --to=git@vger.kernel.org outgoing/* Please note that a cover letter isn't needed unless you're sending a patch series, i.e. unless you're generating and sending more than a single patch at once. Also, fixing the subject line inside the patch files generated by git-format-patch shouldn't be needed in most cases, and commit summaries should be adjusted/rebased instead, if needed.
"H.Merijn Brand" <linux@tux.freedom.nl> writes: > From the Getopt::Long changes: > ``` > Changes in version 2.55 > ----------------------- > * Fix long standing bug that duplicate options were not detected when > the options differ in case while ignore_case is in effect. > This will now yield a warning and become a fatal error in a future > release. > ``` > > Current version is 2.57 This patch looks like duplicate of https://lore.kernel.org/git/20231116193014.470420-1-tmz@pobox.com/ perhaps independently discovered and worked on. Thanks for caring. One downside of unconditional upgrade of the call is, of course, that it would no longer work for those with older Getopt::Long that did not support the "!" suffix. Fortunately, Getopt::Long 2.33 started shipping with Perl 5.8.1 that is more than 20 years old, so with the series we accepted, we also have a change to bump the required version of Perl from 5.8.0 to 5.8.1 to make it clear that it is deliberate that we drop the support for anything older at the same time.
On Mon, 27 Nov 2023 09:58:52 +0900, Junio C Hamano <gitster@pobox.com> wrote: > "H.Merijn Brand" <linux@tux.freedom.nl> writes: > > > From the Getopt::Long changes: > > ``` > > Changes in version 2.55 > > ----------------------- > > * Fix long standing bug that duplicate options were not detected when > > the options differ in case while ignore_case is in effect. > > This will now yield a warning and become a fatal error in a future > > release. > > ``` > > > > Current version is 2.57 > > This patch looks like duplicate of > > https://lore.kernel.org/git/20231116193014.470420-1-tmz@pobox.com/ > > perhaps independently discovered and worked on. Thanks for caring. > > One downside of unconditional upgrade of the call is, of course, > that it would no longer work for those with older Getopt::Long that > did not support the "!" suffix. Fortunately, Getopt::Long 2.33 > started shipping with Perl 5.8.1 that is more than 20 years old, so > with the series we accepted, we also have a change to bump the > required version of Perl from 5.8.0 to 5.8.1 to make it clear that > it is deliberate that we drop the support for anything older at the > same time. The is a no-issue ... Just the 'use Getopt::Long' is enough to guarantee a working version: The '!' was already implemented in version 2.10 (April 1997): --8<--- =item ! Option does not take an argument and may be negated, i.e. prefixed by "no". E.g. "foo!" will allow B<--foo> (with value 1) and B<-nofoo> (with value 0). The option variable will be set to 1, or 0 if negated. -->8--- Looking at the ChangeLog, a reliable behavior of '!' was available since version 2.22 (march 2000): --8<--- Changes in version 2.22 ----------------------- * Fixes a bug in the combination of aliases and negation. Old: "foo|bar!" allowed negation on foo, but not on bar. New: "foo|bar!" allows negation on foo and bar. Caveat: "foo|f!", with bundling, issues the warning that negation on a short option is ignored. To obtain the desired behaviour, use "foo!" => \$opt_foo, "f" => \$opt_foo or "foo|f" => \$opt_foo, "nofoo" => sub { $opt_foo = 0 } Remember that this is _only_ required when bundling is in effect. -->8---
On Sat, Nov 25, 2023 at 10:45:22AM +0100, H.Merijn Brand wrote: > As I am used to PR's by now on all OSS projects I am involved in, or > use git commits or merges directly on the repo, I *never* use > format-patch and/or send-email. > > These docs - yes I read them - do not offer a concise cut-n-paste > example for people like me. In order to have my relative simple patch > submitted (I already had the PR ready, but that came with a huge > warning that PR's are not accepted) I did it the way I did it. Now I > need to read and learn two new commands> I don't think that is very > user-friendly, but that might be just me. These days you can use GitGitGadget to submit a PR to the mailing list: https://gitgitgadget.github.io/ The PR template mentions this, as well as the "about" text for git/git, but I won't be surprised if there are other spots that should be updated to mention it. If you found a message that would benefit from mentioning it, let us know so we can update it. Thanks. -Peff
Hi, H.Merijn Brand wrote: > On Mon, 27 Nov 2023 09:58:52 +0900, Junio C Hamano <gitster@pobox.com> wrote: >> One downside of unconditional upgrade of the call is, of course, >> that it would no longer work for those with older Getopt::Long that >> did not support the "!" suffix. Fortunately, Getopt::Long 2.33 >> started shipping with Perl 5.8.1 that is more than 20 years old, so >> with the series we accepted, we also have a change to bump the >> required version of Perl from 5.8.0 to 5.8.1 to make it clear that >> it is deliberate that we drop the support for anything older at the >> same time. > > The is a no-issue ... > > Just the 'use Getopt::Long' is enough to guarantee a working version: > > The '!' was already implemented in version 2.10 (April 1997): > --8<--- > =item ! > > Option does not take an argument and may be negated, i.e. prefixed by > "no". E.g. "foo!" will allow B<--foo> (with value 1) and B<-nofoo> > (with value 0). > The option variable will be set to 1, or 0 if negated. > -->8--- The real issue is the lack of support for the '--no-' prefix when used with the '!' parameter. The '--no-' form is what has always been documented by git-send-email(1). It was not supported until Getopt::Long 2.33, included in perl 5.8.1. Prior, 'foo!' provided --foo and --nofoo but not --no-foo. But as Junio said, we can accept requiring a perl which was released sometime in the past 2 decades in order to run the most recent git release. ;) Thanks for noticing this and sending a patch!
From 206ace60f7045e309e506a1b9de775f4e9a43b46 Mon Sep 17 00:00:00 2001
From: "H.Merijn Brand - Tux" <linux@tux.freedom.nl>
Date: Fri, 24 Nov 2023 10:27:35 +0100
Subject: [PATCH] perl Getopt::Long now issues warnings for duplicate options
$ perl -Iperl git-send-email.perl.org --help
Duplicate specification "no-validate" for option "no-validate"
Duplicate specification "to-cover|to-cover!" for option "to-cover"
Duplicate specification "no-signed-off-cc|no-signed-off-by-cc" for option "no-signed-off-cc"
Duplicate specification "no-signed-off-cc|no-signed-off-by-cc" for option "no-signed-off-by-cc"
Duplicate specification "no-format-patch" for option "no-format-patch"
Duplicate specification "cc-cover|cc-cover!" for option "cc-cover"
Duplicate specification "no-annotate" for option "no-annotate"
Duplicate specification "no-chain-reply-to" for option "no-chain-reply-to"
Duplicate specification "no-cc-cover" for option "no-cc-cover"
"option!" => \$value
*automatically* supports both --option and --no-option and --nooption
The argument specification can be
! The option does not take an argument and may be negated by
prefixing it with "no" or "no-". E.g. "foo!" will allow "--foo" (a
value of 1 will be assigned) as well as "--nofoo" and "--no-foo" (a
value of 0 will be assigned). If the option has aliases, this
applies to the aliases as well.
Using negation on a single letter option when bundling is in effect
is pointless and will result in a warning.
Signed-off-by: H.Merijn Brand - Tux <linux@tux.freedom.nl>
---
git-send-email.perl | 14 ++------------
1 file changed, 2 insertions(+), 12 deletions(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index d24e981d61..125f49cd08 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -491,7 +491,6 @@ sub config_regexp {
"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,
@@ -506,36 +505,27 @@ sub config_regexp {
"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,
"header-cmd=s" => \$header_cmd,
"no-header-cmd" => \$no_header_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},
+ "cc-cover!" => \$cover_cc,
+ "to-cover!" => \$cover_to,
"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,
--
2.42.1