Fixed passing -0 to tail. Closes #10191

[trypsynth]

No description provided.

[github-actions]

GNU testsuite comparison:

Skipping an intermittent issue tests/shuf/shuf-reservoir (passes in this run but fails in the 'main' branch)
Skipping an intermittent issue tests/sort/sort-stale-thread-mem (passes in this run but fails in the 'main' branch)

[cakebaker]

Can you please add a test to ensure we don't regress in the future? Thanks.

[trypsynth]

@cakebaker all done

[codspeed-hq]

Merging this PR will degrade performance by 27.39%

❌ 9 regressed benchmarks
✅ 273 untouched benchmarks
⏩ 38 skipped benchmarks¹

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

	Mode	Benchmark	BASE	HEAD	Efficiency
❌	Memory	cp_large_file[16]	115.3 KB	119.8 KB	-3.76%
❌	Memory	sort_unique_locale[500000]	33.6 MB	39.8 MB	-15.51%
❌	Memory	sort_ascii_only[500000]	22.2 MB	28.3 MB	-21.77%
❌	Memory	sort_long_line[160000]	712.6 KB	981.4 KB	-27.39%
❌	Memory	sort_key_field[500000]	47.8 MB	51.8 MB	-7.62%
❌	Memory	sort_mixed_data[500000]	22.9 MB	27.1 MB	-15.71%
❌	Memory	sort_numeric[500000]	75.5 MB	79.2 MB	-4.67%
❌	Memory	sort_ascii_utf8_locale	6.2 MB	6.7 MB	-7.35%
❌	Memory	sort_accented_data[500000]	22.1 MB	28.3 MB	-21.79%

---

_{Comparing trypsynth:tail-zero (66a1944) with main (a1545ef)}

Footnotes

1. 38 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

[ChrisDryden]

Could this be an integration test in test_tail.rs? I think it would be more ideal to make sure the behaviour with that specific command doesn't regress, also would have less boilerplate to set up the test conditions

[trypsynth]

@ChrisDryden Done

[github-actions]

GNU testsuite comparison:

Skip an intermittent issue tests/tty/tty-eof (fails in this run but passes in the 'main' branch)

[ChrisDryden]

My bad, I should have been more in-depth in the first review to learn more about the different approaches.

I was thinking it might be better to re-use the parsing logic in parse_num:

match (result.value, is_plus) {
    (0, true) => Ok(Signum::PlusZero),
    (0, false) => Ok(Signum::MinusZero),
    (n, true) => Ok(Signum::Positive(n)),
    (n, false) => Ok(Signum::Negative(n)),
}

If you were to add this to the Signum constructor possibly like this:

impl Signum {
    fn new(num: u64, is_plus: bool) -> Self {
        match (is_plus, num) {
            (true, 0) => Self::PlusZero,
            (false, 0) => Self::MinusZero,
            (true, n) => Self::Positive(n),
            (false, n) => Self::Negative(n),
        }
    }
}

Then you can reuse that same logic in from_obsolete_args and parse_num and replace:

let signum = if args.plus {
  Signum::Positive(args.num)
} else {
  Signum::Negative(args.num)
}

I'm just a bit hesitant on adding that unwrap without any error handling. It would also make the code more consistent since in one place you have Negative(0) and the other you have MinusZero

[sylvestre]

@ChrisDryden let's do that in a different PR