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

ls: set default quoting style to literal when not TTY #5553

Merged
merged 5 commits into from Dec 10, 2023

Conversation

RenjiSann
Copy link
Contributor

This PR fixes the quoting behavior of ls when stdout is non-TTY.
Until now, Coreutils' ls used the same default quoting style no matter what. However, GNU ls uses shell escaped quoting when stdout is a TTY, and Literal quoting otherwise.

I had to modify some quoting tests which expected the TTY-flavored output.

In the future, we should test that the output is still correct when running in the terminal by emulating an stdout file descriptor that can fake the is_terminal() behavior.

Fixes #5490

@RenjiSann
Copy link
Contributor Author

I have no Windows machine on which I can test the actual output of ls in a non-TTY, so I don't know if I should fix the tests or add another condition for the default quoting style depending on the host OS.

Can someone verify the GNU ls behavior on Windows ?

@allaboutevemirolive
Copy link
Contributor

I have no Windows machine on which I can test the actual output of ls in a non-TTY, so I don't know if I should fix the tests or add another condition for the default quoting style depending on the host OS.

Can someone verify the GNU ls behavior on Windows ?

After finishing my work today, I'll test this

@RenjiSann
Copy link
Contributor Author

@allaboutevemirolive Any update ? 😁

Copy link
Member

@tertsdiepraam tertsdiepraam left a comment

Choose a reason for hiding this comment

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

Nice! I think this is good enough to be merged, apart from the tests. I think we can assume that Windows will work the same way, so if you can adapt the tests on Windows accordingly (see the CI output) then it should be fine!

If you want a next challenge (for another PR). Here's what the GNU docs say:

You can specify the default value of the --quoting-style option with the environment variable QUOTING_STYLE. If that environment variable is not set, the default value is ‘shell-escape’ when the output is a terminal, and ‘literal’ otherwise.

We don't support that environment variable yet either :)

Copy link

github-actions bot commented Dec 5, 2023

GNU testsuite comparison:

Congrats! The gnu test tests/tail/inotify-dir-recreate is no longer failing!

@RenjiSann
Copy link
Contributor Author

I've updated the windows tests !

If you want a next challenge (for another PR). Here's what the GNU docs say:

You can specify the default value of the --quoting-style option with the environment variable QUOTING_STYLE. If that environment variable is not set, the default value is ‘shell-escape’ when the output is a terminal, and ‘literal’ otherwise.

May I have the link to this ?

@cakebaker
Copy link
Contributor

@RenjiSann https://www.gnu.org/software/coreutils/manual/html_node/Formatting-the-file-names.html

@cakebaker cakebaker merged commit 673093f into uutils:main Dec 10, 2023
53 of 54 checks passed
@cakebaker
Copy link
Contributor

Thanks for your PR :)

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.

ls: Tab produces \t instead of TAB and the output is not sorted correctly.
4 participants