From: Tian Yuchen <a3205153416@gmail.com>
Date: Sun, 15 Mar 2026 17:22:03 GMT
Subject: Re: [PATCH] apply.c: fix -p argument parsing
Message-ID: <eda5f191-7dfa-4bc0-8ab9-225b20a5e88b@gmail.com>
In-Reply-To: <20260309232700.553168-1-mroik@delayed.space>

Hi Mirko,

> +test_expect_success 'git apply -p 1 patch' '
> +	cat >patch <<-\EOF &&
> +		From 90ad11d5b2d437e82d4d992f72fb44c2227798b5 Mon Sep 17 00:00:00 2001

<<-\EOF should swallow the leading tab, but since you're using spaces 
for indentation here, that would result in a space at the beginning of 
every line, right? I think <<\EOF is correct here.

But Junio said you don't need to worry about it, it's all good ;)

---

The rest are just minor flaws (in my opinion) that you can safely ignore:

> +	if (strtol_i(arg, 10, &state->p_value) < 0 || state->p_value < 0)
> +		die("<num> has to be a non-negative integer");

I think something like:

if (strtol_i(arg, 10, &state->p_value) || state->p_value < 0)
die(_("option -p expects a non-negative integer, got '%s'"), arg);

might be a bit better;

> +test_expect_success 'apply fails due to trailing non-digit in -p' '
> +	test_when_finished "rm -rf t test" &&
> +	test_must_fail git apply -p 2q patch
> +'
> +
> +test_expect_success 'apply fails due to negative number in -p' '
> +	test_when_finished "rm -rf t test patch" &&
> +	test_must_fail git apply -p -1 patch
> +'
> +
>   test_expect_success 'apply git diff with -p2' '
>   	cp file1.saved file1 &&
>   	git apply -p2 patch.file

The 'patch' is created in the first test case, will it prevent the 
subsequent test cases from running on their own?

Overall, the patch itself looks good to me. Thank you!

Regards,

Yuchen