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

OsString Debug implementation prints escaped single quotes #114583

Open
tamird opened this issue Aug 7, 2023 · 1 comment
Open

OsString Debug implementation prints escaped single quotes #114583

tamird opened this issue Aug 7, 2023 · 1 comment
Labels
T-libs Relevant to the library team, which will review and decide on the PR/issue.

Comments

@tamird
Copy link
Contributor

tamird commented Aug 7, 2023

fn main() {
    let string = "'";
    assert_eq!(
        format!("{:?}", string),
        format!("{:?}", std::ffi::OsString::from(string))
    );
}

produces

thread 'main' panicked at 'assertion failed: `(left == right)`
  left: `"\"'\""`,
 right: `"\"\\'\""`', src/main.rs:3:5

(playground)

#83046 reported the behavior of <str as Debug>::fmt escaping single quotes, and it was changed in #83079. Should we do the same for OsString?

I ran into this in #114132. It seems easy to fix for unix:

diff --git a/library/core/src/str/lossy.rs b/library/core/src/str/lossy.rs
index 59f873d1268..7e5ae364361 100644
--- a/library/core/src/str/lossy.rs
+++ b/library/core/src/str/lossy.rs
@@ -1,3 +1,4 @@
+use crate::char::EscapeDebugExtArgs;
 use crate::fmt;
 use crate::fmt::Formatter;
 use crate::fmt::Write;
@@ -85,7 +86,11 @@ fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
                 let valid = chunk.valid();
                 let mut from = 0;
                 for (i, c) in valid.char_indices() {
-                    let esc = c.escape_debug();
+                    let esc = c.escape_debug_ext(EscapeDebugExtArgs {
+                        escape_grapheme_extended: true,
+                        escape_single_quote: false,
+                        escape_double_quote: true,
+                    });
                     // If char needs escaping, flush backlog so far and write, else skip
                     if esc.len() != 1 {
                         f.write_str(&valid[from..i])?;

But on windows OsString is implemented using Wtf8, which is in std rather than core, which means it can't call char::escape_debug_ext:

for c in s.chars().flat_map(|c| c.escape_debug()) {

@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Aug 7, 2023
@fee1-dead fee1-dead added the T-libs Relevant to the library team, which will review and decide on the PR/issue. label Aug 8, 2023
@Noratrieb Noratrieb removed the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Aug 22, 2023
@tamird
Copy link
Contributor Author

tamird commented Sep 20, 2023

Tagging some of the folks involved in #83046 and #83079 for guidance @marcianx @osa1 @m-ou-se.

tamird added a commit to tamird/rust that referenced this issue Nov 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

4 participants