-
Notifications
You must be signed in to change notification settings - Fork 13.2k
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
Add doc aliases for print macros #79232
Conversation
(rust_highfive has picked a reviewer for you, use r? to override) |
I've added |
"echo" for PHP and Bash. "print" for PHP and Lua, "printf" for C and C++, "console.log" for JavaScript
It seems we're having a doc failure here; I can't render docs locally -- so I'm not sure what the right ordering here ought to be so I can make the tests pass. ErrorChecking "macro-print" ... FAILED
==> '{"path":"std","name":"dbg"}' was supposed to be before '{"crate":"std","name":"dbg","path":"std","desc":"Prints and returns the value of a given expression for…","ty":14,"type":null,"is_alias":true,"alias":"macro:print","displayPath":"<span>std::</span>","fullPath":"<span>std::</span>dbg","href":"../std/macro.dbg.html"}'
==> '{"path":"std","name":"eprint"}' was supposed to be before '{"crate":"std","name":"eprint","path":"std","desc":"Prints to the standard error.","ty":14,"type":null,"is_alias":true,"alias":"macro:print","displayPath":"<span>std::</span>","fullPath":"<span>std::</span>eprint","href":"../std/macro.eprint.html"}'
==> '{"path":"std","name":"println"}' was supposed to be before '{"crate":"std","name":"println","path":"std","desc":"Prints to the standard output, with a newline.","ty":14,"type":null,"is_alias":true,"alias":"macro:print","displayPath":"<span>std::</span>","fullPath":"<span>std::</span>println","href":"../std/macro.println.html"}'
==> '{"path":"std","name":"eprintln"}' was supposed to be before '{"crate":"std","name":"eprintln","path":"std","desc":"Prints to the standard error, with a newline.","ty":14,"type":null,"is_alias":true,"alias":"macro:print","displayPath":"<span>std::</span>","fullPath":"<span>std::</span>eprintln","href":"../std/macro.eprintln.html"}' |
Considering it's doc changes, please ping the @rust-lang/docs (or the @rust-lang/rustdoc) teams in the future. Strongly like the idea though! |
@@ -55,6 +55,12 @@ macro_rules! panic { | |||
/// io::stdout().flush().unwrap(); | |||
/// ``` | |||
#[macro_export] | |||
#[doc(alias = "echo")] | |||
#[doc(alias = "print")] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Kinda useless no? :p
@@ -88,6 +94,12 @@ macro_rules! print { | |||
/// println!("format {} arguments", "some"); | |||
/// ``` | |||
#[macro_export] | |||
#[doc(alias = "echo")] | |||
#[doc(alias = "print")] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think it's a good idea to add this alias. They'll find the print
macro which talks about the println
one.
@@ -119,6 +131,12 @@ macro_rules! println { | |||
/// eprint!("Error: Could not complete task"); | |||
/// ``` | |||
#[macro_export] | |||
#[doc(alias = "echo")] | |||
#[doc(alias = "print")] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't add it.
@@ -147,6 +165,12 @@ macro_rules! eprint { | |||
/// eprintln!("Error: Could not complete task"); | |||
/// ``` | |||
#[macro_export] | |||
#[doc(alias = "echo")] | |||
#[doc(alias = "print")] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't add it.
@@ -280,6 +304,13 @@ macro_rules! eprintln { | |||
/// [`debug!`]: https://docs.rs/log/*/log/macro.debug.html | |||
/// [`log`]: https://crates.io/crates/log | |||
#[macro_export] | |||
#[doc(alias = "echo")] | |||
#[doc(alias = "print")] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't add it.
@@ -3,6 +3,7 @@ const QUERY = 'macro:print'; | |||
const EXPECTED = { | |||
'others': [ | |||
{ 'path': 'std', 'name': 'print' }, | |||
{ 'path': 'std', 'name': 'dbg' }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why this change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add more rustdoc-js-std tests for the newly added aliases. You can make multiple tests in one file if you want (take a look at rustdoc-js/doc-alias.js
to see how).
I'm actually having second thoughts about these changes. That together with the process to landing it being more involved than I'd hoped it would be (tests require some effort) I think the best path forward for me here now is to close it. Perhaps someone more motivated than me can pick this back up in the future. |
@GuillaumeGomez I think your review was a lot more strict than it needed to be - I really don't think it makes sense for every |
Why do we need tests for the added aliases? I am confused - we do not generally test documentation in specific, just that a feature works... |
I agree: after talking with other people, it seems a bit unnecessary. The only thing to do in the end is to add the Sorry about the confusion. |
My code comments still need answers/updates though. ;) |
The job Click to see the possible cause of the failure (guessed by this bot)
|
@yoshuawuyts any updates on this? |
@yoshuawuyts Triage: I'm closing this due to inactivity. Feel free to reopen or create a new pr when you have time to work on this again. Thanks! |
Adds a variety of doc aliases for our print macros: "echo" for PHP and Bash. "print" for PHP and Lua, "printf" for C and C++, "console.log" for JavaScript. This only serves to make them easier to find in the doc search. Thanks!