Skip to content

Minor rewrite of env::current_exe docs; clarify symlinks. #46987

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

Merged
merged 1 commit into from
Jan 6, 2018

Conversation

frewsxcv
Copy link
Member

  • Update example in ‘security’ section to use hard links, like the
    linked securityvulns.com example.
  • Weaken language on symbolic links – indicate behavior is
    platform-specific

Fixes #43617.

- Update example in ‘security’ section to use hard links, like the
  linked securityvulns.com example.
- Weaken language on symbolic links – indicate behavior is
  platform-specific

Fixes rust-lang#43617.
@rust-highfive
Copy link
Contributor

r? @TimNN

(rust_highfive has picked a reviewer for you, use r? to override)

///
/// ```bash
/// $ ln foo bar
/// ```
///
/// When you run it, you won't get the original executable, you'll get the
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was the change from ' to deliberate? (Not sure what the convention is here).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

deliberate but not something i feel strongly about. figure the line is already changing so i'm not ruining the diff in any way, but i'm fine keeping as-is

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't mind much either way however it looks like ' is used a lot more than in the existing doc comments, so my personal vote would be to keep ' for consistency (but feel free to keep this as-is to get a second opinion).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is the preferred symbol to use for apostrophe in english text according to the Unicode standard. ' is more commonplace because people can’t always be bothered to type Unicode symbols.

My personal vote is to follow the preferences set forth by the standard (I myself try to type unicode ), but is not something I would bug other people about whether they do or don’t follow the practice.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In all the docs i've written, i've just used the regular ' because is impossible to type on US-layout keyboards without a software compose key or other kind of software text replacement. On the other hand, the difference is likely too small to notice unless it was inconsistent within a single page. So here it's probably fine, but i wouldn't go using it on other pages without grabbing all the others on the same page like you did here. (And then possibly creating a tidy lint for it so we can keep on top of it. >_>)

@TimNN
Copy link
Contributor

TimNN commented Dec 25, 2017

Looks good to me apart from the comment, however I'm unfamiliar with the current style conventions around Rust docs, so

r? @steveklabnik

@rust-highfive rust-highfive assigned steveklabnik and unassigned TimNN Dec 25, 2017
@kennytm kennytm added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. A-docs Area: Documentation for any part of the project, including the compiler, standard library, and tools labels Dec 27, 2017
@alexcrichton
Copy link
Member

ping @steveklabnik (or @rust-lang/docs), mind taking a look?

@GuillaumeGomez
Copy link
Member

Seems good for me.

@QuietMisdreavus
Copy link
Member

Looks good to me. The comment about ' versus is not worth holding up the docs update for.

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Jan 5, 2018

📌 Commit 17380f2 has been approved by QuietMisdreavus

GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Jan 6, 2018
…ietMisdreavus

Minor rewrite of env::current_exe docs; clarify symlinks.

- Update example in ‘security’ section to use hard links, like the
  linked securityvulns.com example.
- Weaken language on symbolic links – indicate behavior is
  platform-specific

Fixes rust-lang#43617.
bors added a commit that referenced this pull request Jan 6, 2018
Rollup of 5 pull requests

- Successful merges: #46987, #47165, #47173, #47202, #47216
- Failed merges:
@bors bors merged commit 17380f2 into rust-lang:master Jan 6, 2018
@frewsxcv frewsxcv deleted the frewsxcv-current-exe branch February 26, 2018 03:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-docs Area: Documentation for any part of the project, including the compiler, standard library, and tools S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants