search

Kobzol / cargo-remark

Cargo subcommand for viewing LLVM optimization remarks.
MIT License
171 stars 2 forks source link

broken links in inline remarks #2

Closed ottojo closed 1 year ago

ottojo commented 1 year ago

Some relative URLs in links inside remarks are broken and contain an invalid src/ prefix.

On page .../target/remarks/web/src/src_day_08_mod.rs.html: 

<div>load of type i32 not eliminated in favor of <a href="src/src_day_08_mod.rs.html#L18">load</a> because ...</a></div>

This link leads to (the non-existing) .../target/remarks/web/src/src/src_day_08_mod.rs.html#L18 instead of correct .../target/remarks/web/src/src_day_08_mod.rs.html#L18

Kobzol commented 1 year ago

Hi! Do you perhaps have an open-source repo where I could try it? Also, could you please send me your rustc version? Thanks!

ottojo commented 1 year ago

I pushed the relevant code reproducing the issue here https://github.com/ottojo/cargo-remark-example, i am using rustc 1.73.0-nightly (1b198b3a1 2023-08-13).

I had a quick look at the code yesterday, and it seems the paths are all correctly extracted (relative to the crate directory), but path_to_relative_url adds the src prefix every time, when it should only do this for the index page?

Kobzol commented 1 year ago

Thanks a lot for the repro!

Ooh, the links inside the actual source files have to be relative to the src directory, not the root directory. This is especially annoying, because the links are contained inside rich string messages that have been prerendered both for the remark list and for the actual inline remarks on source file pages to save memory. I'm thinking of rewriting the static website as a dynamic website with a Rust backend server to resolve these annoyances.

Anyway, does this branch solve the problem on your side? :)

$ cargo install --git https://github.com/Kobzol/cargo-remark --branch fix-relative-links
ottojo commented 1 year ago

Yes, #3 fixes this issue for me, thank you very much!

Serving the pages via HTTP instead of opening a file://... url in the browser would probably also allow to just have /src/... links, since those would then be relative to the hostname, i guess?

Anyways, really nice work with this tool, I wanted to try it immediately after reading your blog post, also having watched the cppcon talk some time ago... I absolutely agree with your conclusion of the limited usefullness of the remarks right now and the outlook on rust-specific optimization remarks. I look forward to some day having optimization remarks in rust of the same quality as compiler errors have improved over the usual c++ experience :smile:

Kobzol commented 1 year ago

Thanks for confirming that it fixes the issue. I have released version 0.1.1 with the fix.