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

printf: support %q #5472

Closed
wants to merge 12 commits into from
Closed

printf: support %q #5472

wants to merge 12 commits into from

Conversation

Luv-Ray
Copy link
Contributor

@Luv-Ray Luv-Ray commented Oct 29, 2023

fix #5468
Now printf %q '"$test"' can produce the right result:

./target/debug/printf %q '"$test"'; echo
\"\$test\"

However I have a doubt about the description in GNU manual:

An additional directive %q, prints its argument string in a format that can be reused as input by most shells. Non-printable characters are escaped with the POSIX proposed $'' syntax, and shell metacharacters are quoted appropriately. This is an equivalent format to ls --quoting=shell-escape output.

And in my local:

$ ls --quoting=shell-escape
'"$test"'
$ printf %q '"$test"'; echo
\"\$test\"

What does "equivalent format" mean?

@github-actions
Copy link

GNU testsuite comparison:

Congrats! The gnu test tests/touch/relative is no longer ERROR!

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.

That is indeed curious. This does seem to give the same output as ls. I think echo might be doing the escaping in your example:

env printf %q '"$test"'

I'd like this to use the same function as in ls because we have a pretty good robust implementation of it there. The implementation of it is here: https://github.com/uutils/coreutils/blob/main/src/uucore/src/lib/features/quoting_style.rs

Note too that there is a little spelling error (the word "metacharacters" needs to be ignored).

@Luv-Ray
Copy link
Contributor Author

Luv-Ray commented Nov 2, 2023

Sorry for the mistake, actually I'm not using the GNU's printf when I directly call printf, which has made me confused.

$ printf %q '$'; echo
\$
$ /usr/bin/printf %q '$'; echo
'$'
$ env printf %q '$'; echo
'$'

There is an echo because printf doesn't add an \n at the end of the output, and It's not the cause of the difference between commands.

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.

Looking good, thanks! Accidentally using the shells printf happens to me all the time too 😄

src/uucore/src/lib/features/tokenize/sub.rs Outdated Show resolved Hide resolved
@@ -261,7 +261,6 @@ impl SubParser {
}
x if legal_fields.binary_search(&x).is_ok() => {
self.field_char = Some(ch);
self.text_so_far.push(ch);
Copy link
Contributor Author

@Luv-Ray Luv-Ray Nov 2, 2023

Choose a reason for hiding this comment

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

This unnecessary push cause wrong error message: printf: %7bb: invalid conversion specification\n, which has one more b.

Copy link

github-actions bot commented Nov 2, 2023

GNU testsuite comparison:

Congrats! The gnu test tests/rm/rm2 is no longer failing!
Congrats! The gnu test tests/tail/inotify-dir-recreate is no longer failing!
Skip an intermittent issue tests/rm/rm1
GNU test failed: tests/tail/retry. tests/tail/retry is passing on 'main'. Maybe you have to rebase?

@Luv-Ray Luv-Ray closed this Nov 9, 2023
@Luv-Ray Luv-Ray deleted the fix-printf-issue5468 branch November 9, 2023 02:12
@Luv-Ray Luv-Ray mentioned this pull request Nov 9, 2023
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.

uu-printf: %q: invalid conversion specification
2 participants