-
Notifications
You must be signed in to change notification settings - Fork 13.3k
Improve std os documentation #2 #14713
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
Improve std os documentation #2 #14713
Conversation
/// ```rust | ||
/// let key = "KEY"; | ||
/// let value = "VALEUR"; | ||
/// std::os::setenv(key, value); |
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.
These two can be written as just os::...
with a use std::os;
added at the top.
I rebased this for clarity. |
@@ -80,7 +80,7 @@ static BUF_BYTES : uint = 2048u; | |||
/// | |||
/// ```rust | |||
/// // We assume that we are in a valid directory like "/home". | |||
/// let current_working_directory = std::os::getcwd(); | |||
/// let current_working_directory = os::getcwd(); |
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.
Does this compile? I think you will need to add use std::os;
to the example.
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.
(You can test these by running make check-stage2-doc-crate-std TESTNAME=os
. After doing that, you can add the NO_REBUILD=1
flag when rerunning that make
invocation, to just run the documentation tests directly, without doing the full bootstrap.)
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.
Thanks you so much for the compilation trick you saved my time! ;)
@alexcrichton : r? |
Thanks for reviews and corrections! |
} | ||
}) | ||
fn _homedir() -> Option<Path> { | ||
aux_homedir("USERPROFILE") |
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.
@alexcrichton If I change this like to
aux_homedir("USERPROFILE").or(aux_homedir("HOME"))
That will make the job?
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.
The old code has the opposite behavior, favoring HOME over USERPROFILE
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.
Thanks for your sharp eyes! It's better to express it in this way? Or the old one?
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.
Either way is fine.
@alexcrichton All green ! |
@alexcrichton r? I think it's the good one. However builds on travis failed but not on my laptop. |
… r=alexcrichton Improving documentation, consistency, removes evils empty lines etc...
@alexcrichton @huonw: Hurray! Thanks for everything I learned a lot about dealing with review, git stuff. :) |
…raw-strings, r=Veykril fix: ide: do not highlight escapes in raw strings fixes rust-lang#14688
Improving documentation, consistency, removes evils empty lines etc...