Formatting/debug of OS strings

[SimonSapin]

OsStr implements fmt::Debug:

• On Unix, using to_lossy_str, then fmt::Debug::fmt on the resulting String, which escapes all control or non-ASCII characters.
• On Windows, it eventually calls an impl I originally wrote for rust-wft8, which is lossless but only escapes surrogate code points. In particular, it happily prints control characters.

So there are two issues: they should be consistent across platforms, and is it OK for a Debug impl to be lossy?

CC @aturon, @alexcrichton

[SimonSapin]

Also, std::path::Display is pointless if impl fmt::Debug for Path does the same.

[alexcrichton]

they should be consistent across platforms

Yes, I think this was just an oversight in the initial implementation.

is it OK for a Debug impl to be lossy?

Yes.

Also, std::path::Display is pointless if impl fmt::Debug for Path does the same.

They're subtly different in that Debug will place quotes around Path but Display does not. Everything basically uses to_string_lossy but the Debug vs Display distinction is whether the Debug or Display implementation is used on the resulting String

[SimonSapin]

Ok. Then impl fmt::Debug for Wtf8 should be changed to use to_string_lossy.

[huonw]

Is all that is required to close this issue just changing

rust/src/libstd/sys/common/wtf8.rs

Lines 388 to 419 in 64a8ffe

	fn fmt(&self, formatter: &mut fmt::Formatter) -> fmt::Result {
	fn write_str_escaped(f: &mut fmt::Formatter, s: &str) -> fmt::Result {
	use fmt::Write;
	for c in s.chars().flat_map(|c| c.escape_default()) {
	try!(f.write_char(c))
	}
	Ok(())
	}
	try!(formatter.write_str("\""));
	let mut pos = 0;
	loop {
	match self.next_surrogate(pos) {
	None => break,
	Some((surrogate_pos, surrogate)) => {
	try!(write_str_escaped(
	formatter,
	unsafe { str::from_utf8_unchecked(
	&self.bytes[pos .. surrogate_pos]
	)},
	));
	try!(write!(formatter, "\\u{{{:X}}}", surrogate));
	pos = surrogate_pos + 3;
	}
	}
	}
	try!(write_str_escaped(
	formatter,
	unsafe { str::from_utf8_unchecked(&self.bytes[pos..]) },
	));
	formatter.write_str("\"")
	}

to something like fmt::Debug::fmt(&self.to_string_lossy(), formatter)?

(I'm tagging as E-easy under that expectation, but it'd be nice to have confirmation.)

[retep998]

I'd personally prefer if the debug implementation on Windows was lossless and had escaped lone surrogates since if I do encounter an OsString that had lone surrogates I'd really like to know what they are and Debug is the tool I'd reach for first.

[brson]

So perhaps we should take the existing code that escapes the surrogates and wrap that in another call to Debug to escape the rest?

[dtolnay]

In my opinion:

• The Debug representation should be consistent across platforms, especially in whether control characters are escaped.
• In general I am perfectly okay with Debug representations being lossy, but in the case of OS strings I believe we can come up with a more helpful representation than to_string_lossy. I agree with @retep998 that pretty much any time you are debugging something with lone surrogates, you really want Debug to show you what they are.

I would be prepared to consider a PR with these changes, and I think we can go from there.

[retep998]

Does the Windows implementation currently print lone surrogates? #46798 only changed how OsStr is printed on Unix platforms.

[Diggsey]

@retep998 the windows implementation always printed lone surrogates.