Skip to content

rustdoc: Fix source-links for files with absolute-paths - #31835

Merged
bors merged 2 commits into
rust-lang:masterfrom
mitaa:rdoc-global-src
Feb 25, 2016
Merged

rustdoc: Fix source-links for files with absolute-paths#31835
bors merged 2 commits into
rust-lang:masterfrom
mitaa:rdoc-global-src

Conversation

@mitaa

@mitaa mitaa commented Feb 23, 2016

Copy link
Copy Markdown
Contributor

@mitaa
mitaa force-pushed the rdoc-global-src branch 2 times, most recently from 53bd2ee to 78ca2e2 Compare February 23, 2016 10:11
Comment thread src/test/run-make/issue-26995/Makefile Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Perhaps this could be a unix-only rustdoc test which has a module with the path as /dev/null? That would avoid run-make and you could still check for the module page, right?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Didn't think of that, but it seems like rustdoc doesn't generate files for empty public modules.

That did make another bug clear though, namely that absolute paths not contained in the root directory are still broken just like before.
(I'm gonna try to put these in a src/__root__ directory; for example /dev/null -> src/__root__/dev/null.html)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hm ok, I just try to avoid run-make like the plague as makefiles are basically impossible to get right on both windows and unix...

@alexcrichton

Copy link
Copy Markdown
Member

@bors: r+ 78ca2e256ed86906f5d295a129f3d51e124dbbeb

@mitaa

mitaa commented Feb 23, 2016

Copy link
Copy Markdown
Contributor Author

I pushed an update so that this also works for absolute paths not contained in the root directory.

That would unfortunately be even worse to test, so I didn't include one. (works locally though)

Comment thread src/librustdoc/html/render.rs Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This seems kinda sketchy, perhaps the iterator below could use components on Path to only look at the Component::Normal parts?

@mitaa

mitaa commented Feb 23, 2016

Copy link
Copy Markdown
Contributor Author

(updated)

I've preserved the Component::ParentDir -> "up" renaming, so this does not look only at Component::Normal. (maybe this can be dropped?)

@alexcrichton

Copy link
Copy Markdown
Member

@bors: r+ 4037a7da77e529a421c4fe32e910f26abacae1af

@bors

bors commented Feb 24, 2016

Copy link
Copy Markdown
Collaborator

⌛ Testing commit 4037a7d with merge 4159981...

@bors

bors commented Feb 24, 2016

Copy link
Copy Markdown
Collaborator

💔 Test failed - auto-win-gnu-32-nopt-t

`clean_srcpath` tries to make the source-path relative to `src_root`,
but this didn't work since `src_root` itself wasn't absolute.
@mitaa

mitaa commented Feb 24, 2016

Copy link
Copy Markdown
Contributor Author

I think I've made your /dev/null suggestion work, removing the run-make test.

This has required commit mitaa@27ca250, because .is_file() returns false for /dev/null.
(the utility of this aside from this test seems questionable, but I hope that's okay..)
(also the same check for extern macros has been used in the same file before here, when emitting source files)

@alexcrichton

Copy link
Copy Markdown
Member

@bors: r+ cf76fcf

Manishearth added a commit to Manishearth/rust that referenced this pull request Feb 25, 2016
bors added a commit that referenced this pull request Feb 25, 2016
@bors
bors merged commit cf76fcf into rust-lang:master Feb 25, 2016
@mitaa
mitaa deleted the rdoc-global-src branch February 25, 2016 20:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

404 for rustdoc [src] links for files with absolute paths

3 participants