df: Move to using OsString
#8371
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
drinkcat
changed the title
OsStringOsString (DRAFT)
|
Just noticed that I need to fix the output too, right now it's mangled: |
|
GNU testsuite comparison: |
|
GNU testsuite comparison: |
This allows us to handle non-Unicode parameters.
This is necessary to fix handling of non-Unicode filepath in `df`. Note that other field might also need to be modified in the future.
Windows handling can be done more properly, but I don't have a machine to test this.
Also pre-record the cell width as getting it back in the printing function would require some conversion back to a String.
Will allow us to write u8 slices directly. Also move the last \n print to the function, simplifying the loop.
Final step, let's just print the raw bytes now.
The tests fail when a non-UTF8 path is mounted, that's... not a common case, but using a lossy string works just as well for the tests, so let's use that.
|
GNU testsuite comparison: |
drinkcat
changed the title
OsString (DRAFT)OsString
|
Should be good to go (just did a little bit of commit message cleanup, and added some tests) |
|
GNU testsuite comparison: |
Kudos, and thanks! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #8357 (there's... a third part,
basenamehas been fixed already)I can't easily add a unit test for
df --output=target "$mnt"though, as that require mounting a filesystem in a weird location, but at least somefsexttest should cover some part of that.The Windows implementation needs to be fixed as well, but at least it's not worse now, than it used to be.
Quite a bit of surgery in
dfneeded to make it print bytes...df: table: Add test for non-Unicode printing
test_df: Use lossy stdout string
The tests fail when a non-UTF8 path is mounted, that's... not a
common case, but using a lossy string works just as well for the
tests, so let's use that.
df: table: Print raw bytes
Final step, let's just print the raw bytes now.
df: table: Move from fmt to a non-standard write_to
Will allow us to write u8 slices directly.
Also move the last \n print to the function, simplifying the loop.
df: table: Collect byte slices for Cell element
Also pre-record the cell width as getting it back in the printing
function would require some conversion back to a String.
uucore: fsext: Fix Windows/Mac
Windows handling can be done more properly, but I don't have
a machine to test this.
fsext: Add parsing test for non-unicode mount point
uucore: fsext: Change MountInfo's mount_root/dir to OsString
This is necessary to fix handling of non-Unicode filepath in
df.Note that other field might also need to be modified in the future.
df: Move to use OsString
This allows us to handle non-Unicode parameters.