From: Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail.com>
Date: Tue, 24 Feb 2026 22:20:36 GMT
Subject: Re: [PATCH v2] send-email: validate charset name in 8bit encoding prompt
Message-ID: <20260224222156.13712-1-shreyanshpaliwalcmsmn@gmail.com>
In-Reply-To: <xmqqqzq9er01.fsf@gitster.g>

> Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail.com> writes:
>
> > When a non-ASCII character is detected in the body or subject of the email
> > the user is prompted with,
> >
> >   Which 8bit encoding should I declare [UTF-8]? foo
> >
> > After this the input string is validated by the regex, based on the fact
> > that the charset string will be minimum 4 characters [1]. If the string is
> > more than 4 letters the email is sent, if not then a second prompt to
> > confirm is asked to the user,
> >
> >   Are you sure you want to use <foo> [y/N]? y
> >
> > This relies on a length based regex heuristic check to validate the user
> > input, and can allow clearly invalid charset names to pass if the input is
> > greater than 4 characters.
> >
> > Add a semantic validation of the charset name using the
> > Encode::find_encoding() module of perl. If the encoding is not recognized,
> > warn the user and ask for confirmation before proceeding. After this
> > validation the lenght based validation becomes redundant and also breaks
> > flow, so change the regex of valid input to any non blank string.
> >
> > Additionally, the wording of the first prompt can confuse the user if not
> > read properly or under any default assumptions for a yes/no prompt. Change
> > the wording to make it explicitly clear to the user that the prompt needs a
> > string input, UTF-8 being the default.
> >
> > The intended flow is,
> >
> >   Declare which 8bit encoding to use [default: UTF-8]? foobar
> >   warning: 'foobar' does not appear to be a valid charset name.
> >   Are you sure you want to use <foobar> [y/N]?
> >
> > [1]- https://github.com/git/git/commit/852a15d748034eec87adbee73a72689c8936fb8b
> >
> > Signed-off-by: Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail.com>
> > ---
> > Changes in v2:
> >  - Added braces in if-else block.
> >
> >  git-send-email.perl   | 17 ++++++++++++++---
> >  t/t9001-send-email.sh |  2 +-
> >  2 files changed, 15 insertions(+), 4 deletions(-)
>
> Curious.  This change to t9001 was there even in the previous
> iteration that did not even work.  How did you test it?

Initially I had the braces in place, when I made the wording change to the failing
test, so all the tests passed. But just before sending the patch, I saw that the
indentation looked a bit off in the annotated git send-email, so I fixed that and
also removed what I thought were unnecessary braces, but I neglected to rerun the
tests after that. My bad.