-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
--filter
argument for split
#1681
Conversation
- add --filter as parameter - carry filter parameter within `Settings` - add `FilterWriter` to expose the stdin of a shell process conveniently as "child process that can be written to" (the process dies when the writer dies)
only unix for now... need to re-write for windows (is sed even available?)
the command needs not to be quoted here. Test was failing because of failing command, although it should've failed earlier (around `succeeds()`)
all characters _in the line_ are changed to 'i'. Newline characters remain unchanged.
fix typo suggested in code review Co-authored-by: Sylvestre Ledru <sledru@mozilla.com>
still missing windows tests and a test with a failed command |
I guess you noticed that Windows is failing |
this uses the TYPE command that somewhat behaves like GNU's `cat`
also, don't use `cat` when running tests in windows
trying out a different "cat for windows" command.
well, right now I'm confused on how to write Is it |
OH MY DAYS Handling I'll change this to a "non-windows"-only feature, then. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. Thanks!
Just some minor comments
Co-authored-by: Sylvestre Ledru <sledru@mozilla.com>
- use `show_error!` instead of `crash!` - cfg-guard code not used in windows builds (remove warnings of unused code)
It was not my intention to modify or to stage this file
src/uu/split/src/split.rs
Outdated
} | ||
} | ||
} | ||
#[cfg(unix)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is a lot of "cfg(unix)"
what about moving that into a specific platform file?
like
https://github.com/uutils/coreutils/tree/master/src/uu/tail/src/platform/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sure, no problem
Co-authored-by: Sylvestre Ledru <sledru@mozilla.com>
Well done :) |
Be able to pass the bytes and filename to a child shell process.