Skip to content

Conversation

@max-amb
Copy link
Contributor

@max-amb max-amb commented Dec 31, 2025

Fixes #9937

Not sure if error message could be better named

@max-amb max-amb marked this pull request as draft December 31, 2025 14:11
@max-amb max-amb marked this pull request as ready for review December 31, 2025 14:25
@max-amb
Copy link
Contributor Author

max-amb commented Dec 31, 2025

I am confused why the windows test doesn't work, the code in unix.rs and windows.rs seems the same to me. Can anyone help?

@oech3
Copy link
Contributor

oech3 commented Jan 1, 2026

csplit of main's code might help

@max-amb
Copy link
Contributor Author

max-amb commented Jan 1, 2026

csplit does this differently, with

        let file = File::open(file_name)
            .map_err_context(|| format!("cannot open {} for reading", file_name.quote()))?;
        Ok(csplit(&options, &patterns, BufReader::new(file))?)

in csplit.rs, 633-635.
It doesn't differentiate between unix and windows as well.

@github-actions
Copy link

github-actions bot commented Jan 1, 2026

GNU testsuite comparison:

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

@github-actions
Copy link

github-actions bot commented Jan 1, 2026

GNU testsuite comparison:

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

@github-actions
Copy link

github-actions bot commented Jan 1, 2026

GNU testsuite comparison:

Skipping an intermittent issue tests/timeout/timeout (passes in this run but fails in the 'main' branch)

@max-amb max-amb requested a review from sylvestre January 2, 2026 11:33
_ => Error::other(
translate!("split-error-unable-to-open-file", "file" => filename),
)
),
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Rust linting in the pre-commit hook doesn't like it not being there, i.e. cargo fmt requires it to be there.

Copy link
Contributor

Choose a reason for hiding this comment

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

OK thanks

@sylvestre sylvestre merged commit 6ac9543 into uutils:main Jan 2, 2026
129 checks passed
sgmarz pushed a commit to sgmarz/coreutils that referenced this pull request Jan 7, 2026
… as directory (uutils#9945)

* split: Added error when attempting to create file that already exists as dir

* split: Added integration test test_split::test_split_directory_already_exists

* Fixed dependency error in windows.rs

* Modified test to work on systems without /dev/zero

* Attempt to fix windows error handling

* Removed test for windows and made it more rigorous

* Err made to look more like gnu

* Updated test to reflect change in err message
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.

split: Does not provide a hint when xaa is a directory

3 participants