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

Tracking issue for the functions for debug escaping char_escape_debug #35068

Closed
tbu- opened this issue Jul 27, 2016 · 20 comments
Closed

Tracking issue for the functions for debug escaping char_escape_debug #35068

tbu- opened this issue Jul 27, 2016 · 20 comments
Labels
B-unstable Blocker: Implemented in the nightly compiler and unstable. C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@tbu-
Copy link
Contributor

tbu- commented Jul 27, 2016

std::char::escape_debug, std::char::EscapeDebug.

@alexcrichton alexcrichton added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. B-unstable Blocker: Implemented in the nightly compiler and unstable. labels Jul 27, 2016
@nikonthethird
Copy link

I hope this is the correct spot to mention that all examples of std::char::escape_debug use std::char::escape_default.

https://doc.rust-lang.org/std/primitive.char.html#method.escape_debug

@Mark-Simulacrum
Copy link
Member

The above has been fixed; the function is correctly documented. Nominating for libs team discussion as to stabilization, I don't think we're going to get much information out of usage since this is such a niche API. I'm personally not certain I like the name: escape_debug seems odd to me, it's not really anything related to debugging I think.

cc @rust-lang/libs

@tbu-
Copy link
Contributor Author

tbu- commented May 16, 2017

debug refers to the fact that it is used for the Debug implementation of String and &str.

I would prefer to call these functions just escape (best would be escape_default, but that one is taken). Could we maybe permanently deprecate escape_default in favor of something like escape_nonascii to make it clear that escaping all non-ASCII code points has nothing to do with "default"?

(I don't suggest ever removing escape_default, just deprecating it.)

@sfackler
Copy link
Member

@rfcbot fcp merge

@rfcbot
Copy link

rfcbot commented May 23, 2017

Team member @sfackler has proposed to merge this. The next step is review by the rest of the tagged teams:

Concerns:

Once these reviewers reach consensus, this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@BurntSushi
Copy link
Member

Sorry, but I've lost context on this change and this issue doesn't appear to link to anything. What is the purpose of this new method? Is it just a rename?

@tbu-
Copy link
Contributor Author

tbu- commented May 27, 2017

The function escapes characters the way the Debug implementation for str and char does.

@dtolnay
Copy link
Member

dtolnay commented Jun 3, 2017

If we are doing this, I would also like to have a plan for #27791 which includes <str>::escape_debug. Looks like there were concerns about consistency between char's and str's escape_default in #27791 (comment).

@rfcbot concern str

@brson
Copy link
Contributor

brson commented Jun 6, 2017

Please change this sentence in the docs: "This will escape the characters similar to the Debug implementations of str or char." I assume this is not just similar, it is the implementation of 'Debug'.

@tbu-
Copy link
Contributor Author

tbu- commented Jun 6, 2017

Currently. What if we wanted to change either of these but not the other? escape_default couldn't be changed because the documentation was too concrete.

@tbu-
Copy link
Contributor Author

tbu- commented Jun 6, 2017

Maybe we could state that it does the same as the Debug implementation for str.

@ariasuni
Copy link
Contributor

ariasuni commented Jun 20, 2017

The only difference between escape_default and escape_debug seems to be that the first uses the range '\x20' ... '\x7e' to determine printable characters (i.e. characters which will not be escaped) where the second uses is_printable(). Why doesn’t it use the actual code for Debug then, instead of this gigantic char_private.rs which is used nowhere else?

@tbu-
Copy link
Contributor Author

tbu- commented Jun 22, 2017

@ariasuni It's the other way around, the Debug implementation of char and str use escape_debug.

@ariasuni
Copy link
Contributor

Oh, that explains a lot. So what does is_printable() is supposed to do?

@tbu-
Copy link
Contributor Author

tbu- commented Jun 22, 2017

It's supposed to decide whether a codepoint is printed directly in the Debug output (e.g. if it's a character from some alphabet), or whether it should be escaped (e.g. if it is a control code point).

is_printable('a')
is_printable('α')
!is_printable('\n')
!is_printable('\0')

@ariasuni
Copy link
Contributor

I get it, I’m wondering on what properties/logic this choice is based. Reading the (undocumented) source code is not helpful… I just understand that it’s based on the Unicode character class (also it probably should be updated for Unicode 10.0.0).

@tbu-
Copy link
Contributor Author

tbu- commented Jun 23, 2017

See the original pull request: #34485.

@dtolnay
Copy link
Member

dtolnay commented Jul 20, 2017

@rfcbot resolved str

char::escape_debug is good to go. We will address the consistency question as part of #27791.

@rfcbot rfcbot added the final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. label Jul 20, 2017
@rfcbot
Copy link

rfcbot commented Jul 20, 2017

🔔 This is now entering its final comment period, as per the review above. 🔔

@Mark-Simulacrum Mark-Simulacrum added the C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. label Jul 22, 2017
@bors bors closed this as completed in 4c9c6e8 Jul 27, 2017
alexcrichton added a commit to alexcrichton/rust that referenced this issue Aug 12, 2017
Stabilizes:

* `<char>::escape_debug`
* `std::char::EscapeDebug`

Closes rust-lang#35068
@leo60228
Copy link
Contributor

I'm not sure of a better place to put this. Is this function guaranteed so that format!("\"{}\"", x.escape_debug()) returns valid Rust code that represents a value equal to x?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
B-unstable Blocker: Implemented in the nightly compiler and unstable. C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests