Re: [Outreachy PATCH v3 2/2] git.c: drop NO_PARSEOPT from builtins that use parse-options

From: Tanga Joris <tangajoris@gmail.com>
Date: Nov 2, 2026, 02:26 UTC
Message-ID: <CAKrRMNogw6rUsZzDsEPLmLAmjB6fvcH4njga+_qBz554x8oEwg@mail.gmail.com>
In-Reply-To: <d026d6a0-5b1b-4bbf-bd1f-5f41c9c2a4e4@gmail.com>

Hello Kaartic,

Thanks for reviewing my patch 2/2, and for writing the test sketch.

On Fri, 30 Oct 2026 at 13:21, Kaartic Sivaraam <kaartic.sivaraam@gmail.com> wrote:

> This looks good to me. I was wondering if it would be valuable to also
> add a test so that any future commands that start using parse_options()
> could be caught if the corresponding change does not also remove
> NO_PARSEOPT from the command?

I also think the test is a good idea. It would definitely catch future commands that switch to parse-options but forget to remove NO_PARSEOPT.

I tried your sketch on top of this series. Without 2/2, it fails for diff-pairs, fast-import and patch-id, so it catches the bug this patch fixes. With patch 2/2, it passes. I had to remove the 'false &&' line to make it run, which I think was left over from testing.

It does not catch var, though, because var only has short options (like -l), and the test looks for lines starting with '--'.

Currently, I am not sure how to make the test catch commands with only short options, because the helper only lists those with long options. Do you have an idea on how we could do this?

Would you like the test inside 2/2, or as a separate patch 3/3?

Thanks, Tanga