[Outreachy PATCH] t1402: test --no-allow-onelevel option precedence

[Outreachy PATCH] t1402: test --no-allow-onelevel option precedence

[Fien Dora]

Git allows later command-line options to override earlier ones.
Added tests to verify that --no-allow-onelevel overrides
--allow-onelevel and vice versa.

This ensures that the expected behavior is preserved if the
argument-parsing code changes in the future.

Signed-off-by: Fien Dora <fiendora5@gmail.com>
---
 t/t1402-check-ref-format.sh | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/t/t1402-check-ref-format.sh b/t/t1402-check-ref-format.sh
index 9dd64662b2..6036df0210 100755
--- a/t/t1402-check-ref-format.sh
+++ b/t/t1402-check-ref-format.sh
@@ -74,6 +74,8 @@ invalid_ref 'heads/foo*/bar*' --refspec-pattern
 ref='foo'
 invalid_ref "$ref"
 valid_ref "$ref" --allow-onelevel
+invalid_ref "$ref" '--allow-onelevel --no-allow-onelevel'
+valid_ref "$ref" '--no-allow-onelevel --allow-onelevel'
 invalid_ref "$ref" --refspec-pattern
 valid_ref "$ref" '--refspec-pattern --allow-onelevel'
 invalid_ref "$ref" --normalize
-- 
2.50.1 (Apple Git-155)

Re: [Outreachy PATCH] t1402: test --no-allow-onelevel option precedence

[Junio C Hamano]

Fien Dora <fiendora5@gmail.com> writes:

> Git allows later command-line options to override earlier ones.
> Added tests to verify that --no-allow-onelevel overrides
> --allow-onelevel and vice versa.

There are many other commands that take --[no-]foo options and a
later --no-foo overrides an earlier --foo (and vice versa), and most
of them do use the parse_options() API.  It probably is not worth
our brain cycles to add tests to cover all these other commands to
cover all these options that can be negated in a way similar to this
patch does for the ones based on parse_options().

What makes check-ref-format somewhat special is that it does not use
the parse_options() API to handle command line options, and instead
uses a handcrafted loop to parse them.  Covering the behaviour of
the command with tests may therefore make a bit more sense than all
the others.

> This ensures that the expected behavior is preserved if the
> argument-parsing code changes in the future.

So I would say that we need two changes to the log message:

 * The rationale should highlight why check-ref-format is special
   and deserves to have this pretty common "no- prefix negates"
   behaviour tested (i.e., it is a handcrafted parse loop); and

 * We have immediate plan to rewrite the option parsing logic (i.e.,
   not a vague "if we change it in the fiture").

to sell this change better.

Thanks.

>
> Signed-off-by: Fien Dora <fiendora5@gmail.com>
> ---
>  t/t1402-check-ref-format.sh | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/t/t1402-check-ref-format.sh b/t/t1402-check-ref-format.sh
> index 9dd64662b2..6036df0210 100755
> --- a/t/t1402-check-ref-format.sh
> +++ b/t/t1402-check-ref-format.sh
> @@ -74,6 +74,8 @@ invalid_ref 'heads/foo*/bar*' --refspec-pattern
>  ref='foo'
>  invalid_ref "$ref"
>  valid_ref "$ref" --allow-onelevel
> +invalid_ref "$ref" '--allow-onelevel --no-allow-onelevel'
> +valid_ref "$ref" '--no-allow-onelevel --allow-onelevel'
>  invalid_ref "$ref" --refspec-pattern
>  valid_ref "$ref" '--refspec-pattern --allow-onelevel'
>  invalid_ref "$ref" --normalize

Re: [Outreachy PATCH] t1402: test --no-allow-onelevel option precedence

[Fien Dora]

Thank you for the feedback.

I'll revise the commit message to better explain why check-ref-format
needs these tests, with the rationale highlighting its handcrafted
option-parsing loop and the planned conversion to parse_options(),
then send a v2 patch.

Thank you

On Mon, 26 Oct 2026 at 22:06, Junio C Hamano <gitster@pobox.com> wrote:
>
> Fien Dora <fiendora5@gmail.com> writes:
>
> > Git allows later command-line options to override earlier ones.
> > Added tests to verify that --no-allow-onelevel overrides
> > --allow-onelevel and vice versa.
>
> There are many other commands that take --[no-]foo options and a
> later --no-foo overrides an earlier --foo (and vice versa), and most
> of them do use the parse_options() API.  It probably is not worth
> our brain cycles to add tests to cover all these other commands to
> cover all these options that can be negated in a way similar to this
> patch does for the ones based on parse_options().
>
> What makes check-ref-format somewhat special is that it does not use
> the parse_options() API to handle command line options, and instead
> uses a handcrafted loop to parse them.  Covering the behaviour of
> the command with tests may therefore make a bit more sense than all
> the others.
>
> > This ensures that the expected behavior is preserved if the
> > argument-parsing code changes in the future.
>
> So I would say that we need two changes to the log message:
>
>  * The rationale should highlight why check-ref-format is special
>    and deserves to have this pretty common "no- prefix negates"
>    behaviour tested (i.e., it is a handcrafted parse loop); and
>
>  * We have immediate plan to rewrite the option parsing logic (i.e.,
>    not a vague "if we change it in the fiture").
>
> to sell this change better.
>
> Thanks.
>
> >
> > Signed-off-by: Fien Dora <fiendora5@gmail.com>
> > ---
> >  t/t1402-check-ref-format.sh | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/t/t1402-check-ref-format.sh b/t/t1402-check-ref-format.sh
> > index 9dd64662b2..6036df0210 100755
> > --- a/t/t1402-check-ref-format.sh
> > +++ b/t/t1402-check-ref-format.sh
> > @@ -74,6 +74,8 @@ invalid_ref 'heads/foo*/bar*' --refspec-pattern
> >  ref='foo'
> >  invalid_ref "$ref"
> >  valid_ref "$ref" --allow-onelevel
> > +invalid_ref "$ref" '--allow-onelevel --no-allow-onelevel'
> > +valid_ref "$ref" '--no-allow-onelevel --allow-onelevel'
> >  invalid_ref "$ref" --refspec-pattern
> >  valid_ref "$ref" '--refspec-pattern --allow-onelevel'
> >  invalid_ref "$ref" --normalize

Re: [Outreachy PATCH] t1402: test --no-allow-onelevel option precedence

[Junio C Hamano]

Fien Dora <fiendora5@gmail.com> writes:

> Thank you for the feedback.
>
> I'll revise the commit message to better explain why check-ref-format
> needs these tests, with the rationale highlighting its handcrafted
> option-parsing loop and the planned conversion to parse_options(),
> then send a v2 patch.
>
> Thank you

Please do not top-post.  Your response will come _after_ the lines you
quote from the message you are responding to.

And trim your quote to the parts necessary for bystanders to understand
what is being discussed.  Do not quote the entire message.

Thanks.

Re: [Outreachy PATCH] t1402: test --no-allow-onelevel option precedence

[Fien Dora]

> Please do not top-post.  Your response will come _after_ the lines you
> quote from the message you are responding to.
> And trim your quote to the parts necessary for bystanders to understand
> what is being discussed.  Do not quote the entire message.

Thank you very much, Junio, for your guidance,

I now understand the convention, I will implement it in my future replies

Thank you

On Tue, 27 Oct 2026 at 17:29, Junio C Hamano <gitster@pobox.com> wrote:
>
> Fien Dora <fiendora5@gmail.com> writes:
>
> > Thank you for the feedback.
> >
> > I'll revise the commit message to better explain why check-ref-format
> > needs these tests, with the rationale highlighting its handcrafted
> > option-parsing loop and the planned conversion to parse_options(),
> > then send a v2 patch.
> >
> > Thank you
>
> Please do not top-post.  Your response will come _after_ the lines you
> quote from the message you are responding to.
>
> And trim your quote to the parts necessary for bystanders to understand
> what is being discussed.  Do not quote the entire message.
>
> Thanks.

Re: [Outreachy PATCH] t1402: test --no-allow-onelevel option precedence

[Fien Dora]

> Fien Dora <fiendora5@gmail.com> writes:

> Please do not top-post.  Your response will come _after_ the lines you
> quote from the message you are responding to.
>
> And trim your quote to the parts necessary for bystanders to understand
> what is being discussed.  Do not quote the entire message.

Thank you very much, Junio, for your guidance.

I now understand the convention, I will apply them in my future replies

Thank you

[Outreachy PATCH v2] t1402: test --no-allow-onelevel option precedence

[Fien Dora]

Unlike most Git commands that use the parse_options() API,
check-ref-format relies on a handcrafted loop to parse command-line
options. Although the --no- prefix is commonly used to negate options
in Git, the precedence of --allow-onelevel and --no-allow-onelevel
is not explicitly covered by the existing tests.

Add regression tests for both orders of --allow-onelevel and
--no-allow-onelevel to verify that the last option takes precedence.
The tests confirm that --no-allow-onelevel overrides a preceding
--allow-onelevel, and that --allow-onelevel similarly overrides a
preceding --no-allow-onelevel.

Git's planned work to improve argument parsing includes replacing
suitable handcrafted parsers with existing APIs. These tests
establish check-ref-format's current option precedence behavior
so it can be preserved when its parsing logic is updated.

Signed-off-by: Fien Dora <fiendora5@gmail.com>
---

Changes since v1:
- Explain why check-ref-format's handcrafted parser needs
  explicit option-precedence tests.
- Clarify how the tests relate to Git's planned parsing work.
- No changes to the test code.

 t/t1402-check-ref-format.sh | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/t/t1402-check-ref-format.sh b/t/t1402-check-ref-format.sh
index 9dd64662b2..6036df0210 100755
--- a/t/t1402-check-ref-format.sh
+++ b/t/t1402-check-ref-format.sh
@@ -74,6 +74,8 @@ invalid_ref 'heads/foo*/bar*' --refspec-pattern
 ref='foo'
 invalid_ref "$ref"
 valid_ref "$ref" --allow-onelevel
+invalid_ref "$ref" '--allow-onelevel --no-allow-onelevel'
+valid_ref "$ref" '--no-allow-onelevel --allow-onelevel'
 invalid_ref "$ref" --refspec-pattern
 valid_ref "$ref" '--refspec-pattern --allow-onelevel'
 invalid_ref "$ref" --normalize
-- 
2.50.1 (Apple Git-155)

Re: [Outreachy PATCH v2] t1402: test --no-allow-onelevel option precedence

[Junio C Hamano]

Fien Dora <fiendora5@gmail.com> writes:

> Unlike most Git commands that use the parse_options() API,
> check-ref-format relies on a handcrafted loop to parse command-line
> options. Although the --no- prefix is commonly used to negate options
> in Git, the precedence of --allow-onelevel and --no-allow-onelevel
> is not explicitly covered by the existing tests.
>
> Add regression tests for both orders of --allow-onelevel and
> --no-allow-onelevel to verify that the last option takes precedence.
> The tests confirm that --no-allow-onelevel overrides a preceding
> --allow-onelevel, and that --allow-onelevel similarly overrides a
> preceding --no-allow-onelevel.

Nicely described.

> Git's planned work to improve argument parsing includes replacing
> suitable handcrafted parsers with existing APIs.

I haven't heard of anybody planning to do such a conversion, but do
you mean you are planning to do such a work as a follow-up?  If so,
write it more like "I plan to do X in a separate series".  If not,
please do not make up hallucinated facts.

> Signed-off-by: Fien Dora <fiendora5@gmail.com>
> ---
>
>  t/t1402-check-ref-format.sh | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/t/t1402-check-ref-format.sh b/t/t1402-check-ref-format.sh
> index 9dd64662b2..6036df0210 100755
> --- a/t/t1402-check-ref-format.sh
> +++ b/t/t1402-check-ref-format.sh
> @@ -74,6 +74,8 @@ invalid_ref 'heads/foo*/bar*' --refspec-pattern
>  ref='foo'
>  invalid_ref "$ref"
>  valid_ref "$ref" --allow-onelevel
> +invalid_ref "$ref" '--allow-onelevel --no-allow-onelevel'
> +valid_ref "$ref" '--no-allow-onelevel --allow-onelevel'
>  invalid_ref "$ref" --refspec-pattern
>  valid_ref "$ref" '--refspec-pattern --allow-onelevel'
>  invalid_ref "$ref" --normalize

Re: [Outreachy PATCH v2] t1402: test --no-allow-onelevel option precedence

[Fien Dora]

> I haven't heard of anybody planning to do such a conversion, but do
> you mean you are planning to do such a work as a follow-up?  If so,
> write it more like "I plan to do X in a separate series".  If not,
> please do not make up hallucinated facts.

Thank you, Junio.

I apologise for presenting the possible conversion as though it were
already planned.

I would be very interested in taking on the conversion of
check-ref-format to parse_options() as a follow-up contribution.
Although it would be a learning challenge for me, I am willing to
study the existing implementation and work through it carefully.

I would be grateful for your guidance on whether this would be a
suitable follow-up project and how best to approach it.

In the meantime, I will revise the current commit message to remove
the unsupported statement.

Thank you again for your guidance.

[Outreachy PATCH v3] t1402: test --no-allow-onelevel option precedence

[Fien Dora]

Unlike most Git commands that use the parse_options() API,
check-ref-format relies on a handcrafted loop to parse command-line
options. Although the --no- prefix is commonly used to negate options
in Git, the precedence of --allow-onelevel and --no-allow-onelevel
is not explicitly covered by the existing tests.

Add regression tests for both orders of --allow-onelevel and
--no-allow-onelevel to verify that the last option takes precedence.
The tests confirm that --no-allow-onelevel overrides a preceding
--allow-onelevel, and that --allow-onelevel similarly overrides a
preceding --no-allow-onelevel.

To improve Git's command-line argument parsing, I plan to
explore converting check-ref-format's handcrafted parser to
parse_options() API in a separate patch series. These tests
establish the current option-precedence behavior that the
conversion would need to preserve.

Signed-off-by: Fien Dora <fiendora5@gmail.com>
---

Changes since v2:
- Clarify that converting check-ref-format to parse_options()
  is work I plan to explore in a separate patch series, not
  an existing Git development plan.
- No changes to the tests.

 t/t1402-check-ref-format.sh | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/t/t1402-check-ref-format.sh b/t/t1402-check-ref-format.sh
index 9dd64662b2..6036df0210 100755
--- a/t/t1402-check-ref-format.sh
+++ b/t/t1402-check-ref-format.sh
@@ -74,6 +74,8 @@ invalid_ref 'heads/foo*/bar*' --refspec-pattern
 ref='foo'
 invalid_ref "$ref"
 valid_ref "$ref" --allow-onelevel
+invalid_ref "$ref" '--allow-onelevel --no-allow-onelevel'
+valid_ref "$ref" '--no-allow-onelevel --allow-onelevel'
 invalid_ref "$ref" --refspec-pattern
 valid_ref "$ref" '--refspec-pattern --allow-onelevel'
 invalid_ref "$ref" --normalize
-- 
2.50.1 (Apple Git-155)