Re: [PATCH 3/3] send-email: teach git send-email option to translate aliases

From: Jacob Keller <jacob.e.keller@intel.com>
Date: Aug 9, 2024, 19:12 UTC
Message-ID: <328fb497-d16e-4af1-a816-7b4aeb531504@intel.com>
In-Reply-To: <xmqqttfufzkj.fsf@gitster.g>

On 8/8/2024 2:57 PM, Junio C Hamano wrote:

Show 8 quoted lines

> Jacob Keller <jacob.e.keller@intel.com> writes:
> 
>> From: Jacob Keller <jacob.keller@gmail.com>
>>
>> Add a new "--translate-aliases" option to git send-email which allows
> 
> If you add a thing, it by definition is new, isn't it?
>

Sure.

>> Instead, skip calling GetOptions a second time of --translate-aliases is
> 
> "of"???  "if" perhaps?
>

Yep, a typo.

Show 9 quoted lines

>> set. This has the effect that known options will instead be translated
>> as aliases instead of producing a warning, but this seems like the best
>> trade off of the available options.
> 
> Hmph, I do not quite get why you need such a hack (to be honest, I
> do not quite understand why dump-aliases needs a similar hack,
> either, even though I do understand why identity thing needs a
> special caseing).
>

The problem is that we have pass_through set in the GetOpt::Long::Configure. This is necessary because we want to pass the options we don't understand into git format-patch.

But --dump-aliases and --translate-aliases both do not want to work with any other option. If we put these into the main get options, then the options will be interpreted normally and we then have to specially check every single option to make sure nothing else was provided.

Dump aliases handled this by checking @ARGV and immediately bailing if there were anything remaining after the call to parsing its inner options. This works but does not work for --translate-aliases because it needs to treat all the remaining arguments as aliases.

Show 13 quoted lines

> After GetOptions() returns, usuall we process everything remaining
> on the command line as files that contain messages, right?  But
> before that happens (i.e. anywhere before the while () loop that
> processes elements in @ARGV), you can check if your new option was
> given, and if so you can map contents of @ARGV using %aliases and
> exit, and you are done, no?  Bonus point if you make sure that no
> other options were given, but perhaps there are some strange folks
> who want to use "--to=fu" as an e-mail alias, so instead of
> complaining that "--translate" does not mix with any other options
> when "git send-email --translate --to=fu" was run, giving alias
> translation for "--to=fu" may be a better behaviour for those users
> anyway ;-).
>

I don't think thats right. We want both --dump-aliases and --translate-aliases to reject "--to" ideally.

I didn't come up with the best way to handle that yet, as currently:

 $ git send-email --translate-aliases --to unknown-alias
 --to
 unknown-alias

I did try turning off option pass_through for the first round of get options, but that also seems to not work properly. I can investigate that route and see if I can get it to behave properly. I think we could drop some special casing.

Essentially, we want to avoid parsing any other options entirely, because they aren't compatible with --dump-aliases or --translate-aliases.