Fix [src] hyperlinks in rustdoc #23213

ipetkov · 2015-03-09T05:31:59Z

rustdoc was doubly appending the file name to the path of where to
generate the source files, meanwhile, the [src] hyperlinks were not
Added a flag to rustdoc::html::render::clean_srcpath to ignore the
last path component, i.e. the file name itself to prevent the issue
This also avoids creating directories with the same name as source
files, and it makes sure the link to main.css is correct as well.

rust-highfive · 2015-03-09T05:32:11Z

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

Gankra · 2015-03-09T13:06:38Z

src/librustdoc/html/render.rs

@@ -1,4 +1,4 @@
-// Copyright 2013-2014 The Rust Project Developers. See the COPYRIGHT
+// Copyright 2013-2015 The Rust Project Developers. See the COPYRIGHT


We've basically agreed to stop touching these. They're legal cargo-culting.

@gankro #9070 (comment)

Fair enough.

Gankra · 2015-03-09T13:08:14Z

Would it be possible to write a regression test for this?

steveklabnik · 2015-03-09T18:19:30Z

🎊

tomjakubowski · 2015-03-09T21:19:10Z

@ipetkov if you're not familiar with rustdoc integration/regression testing, look in the src/test/run-make/rustdoc-* directories. They are run with a Python script located at etc/htmldocck.py. In brief, the script parses directives from comments and tries to match text/patterns at a given XPath in an HTML document given by a file path.

You might want to check that a [src] link with some URL exists in an item's document, and then another one that checks that an HTML file exists at the relative path given by that URL.

Gankra · 2015-03-09T21:51:39Z

@tomjakubowski Are you interested in being the unofficial-official reviewer for this? You're the most active in this codebase recently.

ipetkov · 2015-03-10T00:51:52Z

@tomjakubowski I wasn't until today, but it seemed pretty straight forward :)

Also the PR has been updated to include a regression test

tomjakubowski · 2015-03-12T04:24:17Z

@ipetkov looks good to me, but I'd prefer if this regression had its own test. rustdoc-smoke is already vague enough and I'd rather it not get cluttered over time.

I'm also a little bit curious why this bug only cropped up recently. Generating [src] links worked fine (minus some special cases like the alloc crate bug) until a week or so ago and then it broke completely seemingly out of nowhere.

ipetkov · 2015-03-15T05:17:21Z

@tomjakubowski I'm really not sure what caused the breakage, though I have a feeling it might be related with reforming PathBuf because in the past it might not have included the file name when it was converted into an iterator (though I have not looked into it and I could be wrong).

@gankro @tomjakubowski I've updated the PR so that the regression tests are in their own test rather than in rustdoc-smoke and rebased against the updated master.

Sorry for the delay at updating this, my machine suffered from a hardware failure a few nights ago and I haven't had access to my files. I haven't tested (or built) these new updates, though I tried to double check the test cases, and hopefully the build will pass.

huonw · 2015-03-16T23:47:01Z

src/librustdoc/html/render.rs

@@ -695,13 +695,18 @@ fn shortty(item: &clean::Item) -> ItemType {
 /// static HTML tree.
 // FIXME (#9639): The closure should deal with &[u8] instead of &str
 // FIXME (#9639): This is too conservative, rejecting non-UTF-8 paths
-fn clean_srcpath<F>(src_root: &Path, p: &Path, mut f: F) where
+fn clean_srcpath<F>(src_root: &Path, p: &Path, keep_filename: bool, mut f: F) where


Could you add a brief mention of what keep_filename does to the docs?

ipetkov · 2015-03-17T02:49:55Z

@huonw Updated with docs!

huonw · 2015-03-18T06:34:44Z

@bors r+

bors · 2015-03-18T06:34:44Z

📌 Commit cc27f29 has been approved by huonw

bors · 2015-03-18T11:59:32Z

⌛ Testing commit cc27f29 with merge 8165036...

Manishearth · 2015-03-18T12:41:29Z

Test breaks on a mac?

http://buildbot.rust-lang.org/builders/auto-mac-64-nopt-t/builds/4122/steps/test/logs/stdio

----- /Users/rustbuild/src/rust-buildbot/slave/auto-mac-64-nopt-t/build/src/test/run-make/rustdoc-src-links/ --------------------
------ stdout ---------------------------------------------
DYLD_LIBRARY_PATH="/Users/rustbuild/src/rust-buildbot/slave/auto-mac-64-nopt-t/build/obj/x86_64-apple-darwin/test/run-make/rustdoc-src-links:/Users/rustbuild/src/rust-buildbot/slave/auto-mac-64-nopt-t/build/obj/x86_64-apple-darwin/stage2/lib:" /Users/rustbuild/src/rust-buildbot/slave/auto-mac-64-nopt-t/build/obj/x86_64-apple-darwin/stage2/bin/rustdoc -w html -o /Users/rustbuild/src/rust-buildbot/slave/auto-mac-64-nopt-t/build/obj/x86_64-apple-darwin/test/run-make/rustdoc-src-links/doc foo.rs
/System/Library/Frameworks/Python.framework/Versions/2.7/Resources/Python.app/Contents/MacOS/Python /Users/rustbuild/src/rust-buildbot/slave/auto-mac-64-nopt-t/build/src/etc/htmldocck.py /Users/rustbuild/src/rust-buildbot/slave/auto-mac-64-nopt-t/build/obj/x86_64-apple-darwin/test/run-make/rustdoc-src-links/doc foo.rs

------ stderr ---------------------------------------------
Traceback (most recent call last):
  File "/Users/rustbuild/src/rust-buildbot/slave/auto-mac-64-nopt-t/build/src/etc/htmldocck.py", line 396, in <module>
    check(sys.argv[1], get_commands(sys.argv[2]))
  File "/Users/rustbuild/src/rust-buildbot/slave/auto-mac-64-nopt-t/build/src/etc/htmldocck.py", line 389, in check
    c.cmd, c.lineno))
RuntimeError: @has check failed at line 42
make[1]: *** [all] Error 1

bors · 2015-03-18T12:42:38Z

⛄ The build was interrupted to prioritize another pull request.

bors · 2015-03-18T14:23:08Z

⌛ Testing commit cc27f29 with merge a30341f...

bors · 2015-03-18T15:02:36Z

⛄ The build was interrupted to prioritize another pull request.

* rustdoc was doubly appending the file name to the path of where to generate the source files, meanwhile, the [src] hyperlinks were not * Added a flag to rustdoc::html::render::clean_srcpath to ignore the last path component, i.e. the file name itself to prevent the issue * This also avoids creating directories with the same name as source files, and it makes sure the link to `main.css` is correct as well. * Added regression tests to ensure the rustdoc heirarchy of rendered source files remains consistent Fixes #23192

bors · 2015-03-18T22:06:39Z

⌛ Testing commit cc27f29 with merge 10d5cdf...

ipetkov · 2015-03-18T23:00:41Z

@huonw @Manishearth d'oh copy paste error in a test file. I was able to test a fix locally and the update should fix things now!

Manishearth · 2015-03-19T13:15:43Z

@bors: r=huonw

bors · 2015-03-19T13:15:44Z

📌 Commit af6cf85 has been approved by huonw

bors · 2015-03-19T16:42:17Z

⌛ Testing commit af6cf85 with merge 81e2396...

* rustdoc was doubly appending the file name to the path of where to generate the source files, meanwhile, the [src] hyperlinks were not * Added a flag to rustdoc::html::render::clean_srcpath to ignore the last path component, i.e. the file name itself to prevent the issue * This also avoids creating directories with the same name as source files, and it makes sure the link to `main.css` is correct as well. Fixes #23192

bors · 2015-03-19T19:15:20Z

☀️ Test successful - auto-linux-32-nopt-t, auto-linux-32-opt, auto-linux-64-nopt-t, auto-linux-64-opt, auto-linux-64-x-android-t, auto-mac-32-opt, auto-mac-64-nopt-t, auto-mac-64-opt, auto-win-32-nopt-t, auto-win-32-opt, auto-win-64-nopt-t, auto-win-64-opt

rust-highfive assigned huonw Mar 9, 2015

Gankra reviewed Mar 9, 2015
View reviewed changes

huonw reviewed Mar 16, 2015
View reviewed changes

bors merged commit af6cf85 into rust-lang:master Mar 19, 2015

ipetkov deleted the rustdoc-src-fix branch March 19, 2015 20:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix [src] hyperlinks in rustdoc #23213

Fix [src] hyperlinks in rustdoc #23213

ipetkov commented Mar 9, 2015

rust-highfive commented Mar 9, 2015

Gankra Mar 9, 2015

frewsxcv Mar 9, 2015

Gankra Mar 9, 2015

Gankra commented Mar 9, 2015

steveklabnik commented Mar 9, 2015

tomjakubowski commented Mar 9, 2015

Gankra commented Mar 9, 2015

ipetkov commented Mar 10, 2015

tomjakubowski commented Mar 12, 2015

ipetkov commented Mar 15, 2015

huonw Mar 16, 2015

ipetkov commented Mar 17, 2015

huonw commented Mar 18, 2015

bors commented Mar 18, 2015

bors commented Mar 18, 2015

Manishearth commented Mar 18, 2015

bors commented Mar 18, 2015

bors commented Mar 18, 2015

bors commented Mar 18, 2015

bors commented Mar 18, 2015

ipetkov commented Mar 18, 2015

Manishearth commented Mar 19, 2015

bors commented Mar 19, 2015

bors commented Mar 19, 2015

bors commented Mar 19, 2015

		@@ -1,4 +1,4 @@
		// Copyright 2013-2014 The Rust Project Developers. See the COPYRIGHT
		// Copyright 2013-2015 The Rust Project Developers. See the COPYRIGHT

Fix [src] hyperlinks in rustdoc #23213

Fix [src] hyperlinks in rustdoc #23213

Conversation

ipetkov commented Mar 9, 2015

rust-highfive commented Mar 9, 2015

Gankra Mar 9, 2015

Choose a reason for hiding this comment

frewsxcv Mar 9, 2015

Choose a reason for hiding this comment

Gankra Mar 9, 2015

Choose a reason for hiding this comment

Gankra commented Mar 9, 2015

steveklabnik commented Mar 9, 2015

tomjakubowski commented Mar 9, 2015

Gankra commented Mar 9, 2015

ipetkov commented Mar 10, 2015

tomjakubowski commented Mar 12, 2015

ipetkov commented Mar 15, 2015

huonw Mar 16, 2015

Choose a reason for hiding this comment

ipetkov commented Mar 17, 2015

huonw commented Mar 18, 2015

bors commented Mar 18, 2015

bors commented Mar 18, 2015

Manishearth commented Mar 18, 2015

bors commented Mar 18, 2015

bors commented Mar 18, 2015

bors commented Mar 18, 2015

bors commented Mar 18, 2015

ipetkov commented Mar 18, 2015

Manishearth commented Mar 19, 2015

bors commented Mar 19, 2015

bors commented Mar 19, 2015

bors commented Mar 19, 2015