Skip to content

Write the "passes" chapter of the rustdoc book - #43790

Merged
bors merged 1 commit into
rust-lang:masterfrom
steveklabnik:rustdoc-passes
Aug 15, 2017
Merged

Write the "passes" chapter of the rustdoc book#43790
bors merged 1 commit into
rust-lang:masterfrom
steveklabnik:rustdoc-passes

Conversation

@steveklabnik

Copy link
Copy Markdown
Contributor

cc #42322

r? @rust-lang/docs

@QuietMisdreavusQuietMisdreavus left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

When #43348 is merged, will you want to add its propagate-doc-cfg pass? It relies on the #[doc(cfg)] attribute form, which will be behind a feature flag when it's first released.

Comment threadsrc/doc/rustdoc/src/passes.md Outdated

@QuietMisdreavusQuietMisdreavusAug 10, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

In my experience it takes this indentation amount from the smallest indentation it sees over the whole docstring. This is the entire reason #42760 exists. Putting an example with differently-indented lines like below, but with the order reversed, will demonstrate this.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

I took the examples from the tests. Maybe we should add more tests for those cases.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What it seems to do is take the smallest indent from the whole document but it will ignore the indent on the first line if the second line isn't entirely white-space. It also always removes white-space from the beginning of the first line and doesn't remove anything from lines which are entirely white-space. I'm sure the logic can be improved because there are also #38173 and #38739 which I think are also caused by this.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

should we maybe just be more vague about this, rather than going into this level of detail? That way we can tweak it in the future.

@steveklabnik

Copy link
Copy Markdown
ContributorAuthor

When #43348 is merged, will you want to add its propagate-doc-cfg pass? It relies on the #[doc(cfg)] attribute form, which will be behind a feature flag when it's first released.

Hmmm how to handle unstable things is.... something I haven't though of yet.

Comment threadsrc/doc/rustdoc/src/passes.md Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this should be:

```textline1line2```

@steveklabniksteveklabnikAug 11, 2017

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

not according to the tests, unless i made a mistake

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

#[test]
fnshould_ignore_first_line_indent(){
// The first line of the first paragraph may not be indented as
// far due to the way the doc string was written:
//
// #[doc = "Start way over here
// and continue here"]
let s = "line1\n line2".to_string();
let r = unindent(&s);
assert_eq!(r,"line1\nline2");
}

Comment threadsrc/doc/rustdoc/src/passes.md Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

*converted

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

fixed

Comment threadsrc/doc/rustdoc/src/passes.md Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What it seems to do is take the smallest indent from the whole document but it will ignore the indent on the first line if the second line isn't entirely white-space. It also always removes white-space from the beginning of the first line and doesn't remove anything from lines which are entirely white-space. I'm sure the logic can be improved because there are also #38173 and #38739 which I think are also caused by this.

Comment threadsrc/doc/rustdoc/src/passes.md Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actually, this one would also be:

```textline1line2```

However

```text line1 line2```

becomes:

```textline1 line2```

Whether the second line is entirely white-space or not makes a difference.

@shepmastershepmaster added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Aug 11, 2017
@steveklabnik

Copy link
Copy Markdown
ContributorAuthor

I've gone with #43790 (comment), this should be ready for review

@steveklabniksteveklabnik mentioned this pull request Aug 14, 2017
Comment threadsrc/doc/rustdoc/src/passes.md Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

typo in link (missing c)

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

fixed, thanks

@QuietMisdreavus

Copy link
Copy Markdown
Contributor

@bors r+ rollup

❤️

@bors

bors commented Aug 14, 2017

Copy link
Copy Markdown
Collaborator

📌 Commit 7a5ee17 has been approved by QuietMisdreavus

comment syntax from the text of the comment. This pass is what removes that
space.

The exact rules are left under-specified so that we can fix issues that we find.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This line confuses me. Is this pass anything more complex than removing a single space if it exists? Are we not able to fix issues if we specify the rules?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ah, just saw this was approved. Don't block this PR on my comment here, but feel free to answer if there's an answer

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The current semantics are a bit awkward to specify completely, see the thread at #43790 (comment) for a brief summary. I think the thought for stating this was that if we describe everything it does then it would create a situation where either we get penned in by the current implementation, or the docs inevitably get out of sync when we patch around various issues.

frewsxcv added a commit to frewsxcv/rust that referenced this pull request Aug 15, 2017
…tMisdreavus
Write the "passes" chapter of the rustdoc book
cc rust-lang#42322
r? @rust-lang/docs
@frewsxcvfrewsxcv mentioned this pull request Aug 15, 2017
bors added a commit that referenced this pull request Aug 15, 2017
Rollup of 6 pull requests
- Successful merges: #43756, #43790, #43846, #43848, #43862, #43868
- Failed merges:
@bors
bors merged commit 7a5ee17 into rust-lang:masterAug 15, 2017
bors added a commit that referenced this pull request Aug 15, 2017
Ship the rustdoc book
Fixes#42322, as it's the last step.
Blocked on #43790, though they will not conflict.
r? @rust-lang/docs
@steveklabnik
steveklabnik deleted the rustdoc-passes branch October 25, 2017 18:30
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-authorStatus: This is awaiting some action (such as code changes or more information) from the author.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants

@steveklabnik@QuietMisdreavus@bors@durka@frewsxcv@ollie27@shepmaster