Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

csplit: fix bug when --suppress-matched flag is active and positive/negative offset is present #7088

Merged
merged 6 commits into from
Jan 9, 2025

Conversation

Felle33
Copy link
Contributor

@Felle33 Felle33 commented Jan 7, 2025

I modified the test cases test_up_to_match_offset_option_suppress_matched according to the issue #7052 and also the test test_up_to_match_negative_offset_option_suppress_matched because the output was different with the GNU implementation.

The code should fix the bug:
In case of positive offset: the code checks if the offset is greater than 0, so the last line should be skipped
In case of negative offset: when the regex is matched, the first element of the buffer should be deleted and the current line inserted.

…atched according to issue uutils#7052 and modified also test_up_to_match_negative_offset_option_suppress_matched
Copy link

github-actions bot commented Jan 7, 2025

GNU testsuite comparison:

Skip an intermittent issue tests/tail/inotify-dir-recreate (fails in this run but passes in the 'main' branch)

…atched according to issue uutils#7052 and modified also test_up_to_match_negative_offset_option_suppress_matched
@cakebaker cakebaker force-pushed the csplit_suppressed_matched branch from 2796cc1 to c1b9509 Compare January 8, 2025 13:43
Copy link

github-actions bot commented Jan 8, 2025

GNU testsuite comparison:

Congrats! The gnu test tests/misc/stdbuf is no longer failing!

Comment on lines 441 to 446
} else {
// since offset_usize is for sure greater than 0
// the first element of the buffer should be removed and this
// line inserted to be coherent with GNU implementation
input_iter.add_line_to_buffer(ln, l);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A detail: I would swap the if and else blocks. It allows you to get rid of the ! in the condition for better readability.

@cakebaker cakebaker linked an issue Jan 8, 2025 that may be closed by this pull request
@Felle33
Copy link
Contributor Author

Felle33 commented Jan 8, 2025

Thanks, I have swapped the if and else blocks. A curiosity, why did the first CI/CD tests fail?

Copy link

github-actions bot commented Jan 8, 2025

GNU testsuite comparison:

Skip an intermittent issue tests/timeout/timeout (fails in this run but passes in the 'main' branch)
Congrats! The gnu test tests/misc/stdbuf is no longer failing!

@cakebaker cakebaker merged commit 33ac583 into uutils:main Jan 9, 2025
65 checks passed
@cakebaker
Copy link
Contributor

Thanks!

A curiosity, why did the first CI/CD tests fail?

I don't remember what the error was then, sometimes it simply fails randomly :|

@Felle33 Felle33 deleted the csplit_suppressed_matched branch January 10, 2025 16:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

csplit: incorrect split with --suppress-matched and positive offset
2 participants