From: Shashank S Date: Mon, 02 Nov 2026 04:56:04 GMT Subject: Re: [PATCH v2] test-chmtime: use parse_options() to parse command-line flags Message-ID: In-Reply-To: <20261102024623.24897-1-herroyalpriestess@gmail.com> On Mon, 2 Nov 2026 at 08:17, Cynthia Uche wrote: Hi Cynthia, Thank you so much for taking the time to test and review this > > Hi Shashank > No test covers this. With that argument set to 0, t0082 still passed > 3 of 3, while "test-tool chmtime --get -86400 file" failed with > "unknown switch `8'" (exit 129). Could you add a test like this one? > It passes with v2 and fails without the flag: > > test_expect_success 'negative offset is not taken as an option' ' > test-tool chmtime =1000000000 file && > test-tool chmtime --get -86400 file >actual && > echo 999913600 >expect && > test_cmp expect actual > ' > This is an excellent catch. Adding a dedicated test to prevent a regression here is a great idea, and I appreciate you providing the snippet. I will include this test in the v3 iteration. > Compared with the base build, I also saw three changes that the > commit message does not mention: > > - "+0 --get f": base tried to stat "--get"; v2 accepts the flag > after the timespec and prints the mtime. > - "-g -v f": base tried to stat "-v" [and still printed the mtime] > and exited 0; v2 dies (exit 128). > - usage errors exit with 129 instead of 1. > > Could you mention them in the commit message, or let me know if any > of them is not intended? > All of these behavioral shifts are intended! Since `parse_options()` handles arguments independent of their position and uses standard error codes (129 for usage, 128 for `die`), the tool now behaves much more like a standard Git command. You are completely right that I should document these side-effects explicitly in the commit message. I will add a breakdown of these behavioral improvements to the commit message in v3. Thanks again for the meticulous review! Shashank S Mogaveera