Skip to content

join: add support for non-unicode field separators - #2902

Merged
tertsdiepraam merged 1 commit into
uutils:mainfrom
jtracey:join-non-unicode-sep
Jan 31, 2022
Merged

join: add support for non-unicode field separators#2902
tertsdiepraam merged 1 commit into
uutils:mainfrom
jtracey:join-non-unicode-sep

Conversation

@jtracey

@jtracey jtracey commented Jan 21, 2022

Copy link
Copy Markdown
Contributor

This allows for -t to take invalid unicode (but still single-byte) values
on unix-like platforms. Other platforms, which as of the time of this commit
do not support OsStr::as_bytes(), could possibly be supported in the future,
but would require design decisions as to what that means.

See the section on conversions in the ffi Rust docs for more detail on what is currently exposed.

Lets the 8-bit-t GNU test pass (see #2634).

@jtracey jtracey mentioned this pull request Jan 21, 2022
10 tasks
Comment thread tests/by-util/test_join.rs Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

it would be nice to update the test on non unix to verify that the error is here :)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Rebased and pushed with a new test. We have to use Windows for our non-unix, since that's the only other OS that gives any sort of access to the internal OsString representation.

@jtracey
jtracey force-pushed the join-non-unicode-sep branch 4 times, most recently from 984a5ba to 04e5538 Compare January 30, 2022 23:41
This allows for `-t` to take invalid unicode (but still single-byte) values
on unix-like platforms. Other platforms, which as of the time of this commit
do not support `OsStr::as_bytes()`, could possibly be supported in the future,
but would require design decisions as to what that means.
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.

3 participants