Skip to content
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

rustdoc: Please provide way to set the default theme #77024

Closed
ijackson opened this issue Sep 21, 2020 · 17 comments · Fixed by #77213
Closed

rustdoc: Please provide way to set the default theme #77024

ijackson opened this issue Sep 21, 2020 · 17 comments · Fixed by #77213
Labels
A-rustdoc-themes Area: Themes for HTML pages generated by rustdoc C-feature-request Category: A feature request, i.e: not implemented / a PR. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@ijackson
Copy link
Contributor

ijackson commented Sep 21, 2020

When I view rustdoc-generated documents via file:// urls on my own system, the (I think cookie-based) mechanism for saving my doc theme preference does not work.

Can we please have a command-line option or config file setting or something, to change the default theme? (I guess it might be possible to do this with --extend-css but that seems complicated and probably fragile too.)

I imagine implementing this would not be too hard even for someone unfamiliar with the rustdoc code, so I would be happy to do an MR if you like. I guess the right way would be a --default-theme command line option?

@jyn514 jyn514 added T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. A-rustdoc-themes Area: Themes for HTML pages generated by rustdoc C-feature-request Category: A feature request, i.e: not implemented / a PR. labels Sep 21, 2020
@jyn514
Copy link
Member

jyn514 commented Sep 21, 2020

-Z unstable-theme seems reasonable (or -Z unstable-options --default-theme). @Cldfire might have suggestions for how to implement it.

@Mark-Simulacrum
Copy link
Member

Are we sure that there's nothing we can do that'll work for file:// URLs? I guess most things are probably tied to the domain, but maybe something that's global to file:/// exists? I'm not personally familiar with browser APIs that would allow this though.

@jyn514
Copy link
Member

jyn514 commented Sep 21, 2020

Are we sure that there's nothing we can do that'll work for file:// URLs? I guess most things are probably tied to the domain, but maybe something that's global to file:/// exists? I'm not personally familiar with browser APIs that would allow this though.

rust-lang/rustup#2151 (comment)

@ijackson
Copy link
Contributor Author

ijackson commented Sep 21, 2020 via email

@workingjubilee
Copy link
Member

The current solution uses localStorage, which is designed to work in a "per-website" fashion but persist across sessions. However, there are several special-cases around file:// addresses in general for security reasons. I agree that a local filesystem concern should probably not be handled with Web APIs, and offering a compilation option that strips out the JS that handles localStorage and then includes a specific css theme should be implementable, and would probably be the simplest approach.

@Cldfire
Copy link
Contributor

Cldfire commented Sep 22, 2020

@ijackson:

However it seems to me that it should be up to the person who is
building documents for local use, what the default theme is (for a
user who hasn't specified a personal default).

I agree, and we can absolutely do this. See https://github.com/rust-lang/rust/blob/master/src/librustdoc/html/static/storage.js#L126-L128; the call to switchTheme defaults to light if no saved setting or system preference is found. It would be trivial to swap that out for a harcoded theme name based on a CLI arg (or even just change the default from light to something else).

@workingjubilee:

offering a compilation option that strips out the JS that handles localStorage and then includes a specific css theme should be implementable, and would probably be the simplest approach.

The above would be the "simplest" approach, but this would produce the cleaner end result as we'd be able to remove the (non-functioning) theme selector as well.

That would also introduce more maintenance complexity, however.

@workingjubilee
Copy link
Member

...perplexed, did I say something that did not have an identical meaning?

@Cldfire
Copy link
Contributor

Cldfire commented Sep 22, 2020

...perplexed, did I say something that did not have an identical meaning?

Unless I'm misreading, your approach involves removing the storage.js file whereas the one I described does not. This precludes the ability to change the default theme chosen for users who haven't yet specified a default while keeping the theme picker functionality. Other than that, they're definitely very similar 🙂

@workingjubilee
Copy link
Member

Oh, true actually, I crossed a wire at some point.

@Mark-Simulacrum
Copy link
Member

IMO, while a compilation time option is plausible, I would personally prefer that we just make the existing options UI work locally rather than adding ability to configure defaults; that would apply (presumably) to all Rust documentation viewed through file:///, so it shouldn't be too much of a problem to configure that once.

@jyn514
Copy link
Member

jyn514 commented Sep 22, 2020

That would be great to do, but I don't see how it's possible. There's no shared state between local files, rustdoc would need to run a local webserver which has its own issues.

@workingjubilee
Copy link
Member

That's fair.
So, speaking of which: I cannot reproduce this bug.
I used rustup doc and clicked through to TRPL in Firefox. The themes were default (light) on this computer. I set the theme to the "Rust" sepia-ish theme. I then closed Firefox entirely. I then reopened TRPL in the same manner. The theme was still set. I repeated the same process with the std docs, but because they use a different set of themes, I needed to set a new theme.

The actual problem is likely:
User expects setting theme on e.g. Rust By Example to also set theme on e.g. The Rust Programming Language. A reasonable expectation as these both use the same theme set (with std it is slightly touchier).
The actual behavior: Each is determined independently, likely because they are "different sites" insofar as the browser is concerned.

@Cldfire
Copy link
Contributor

Cldfire commented Sep 22, 2020

I set the theme to the "Rust" sepia-ish theme. I then closed Firefox entirely. I then reopened TRPL in the same manner. The theme was still set. I repeated the same process with the std docs, but because they use a different set of themes, I needed to set a new theme.

Hmm, did you try navigating to a different page within TRPL / rustdoc after changing the theme on the first page? At least for me locally, the theme will persist for a given URL once set, but navigating to a different page under a different URL resets it back to the default and requires changing again.

@workingjubilee
Copy link
Member

workingjubilee commented Sep 22, 2020

Oh, you are correct.

Notably: file://{$HOME}/rustup/toolchains/nightly-x86_64-unknown-linux-gnu/share/doc/rust/html/std/primitive.bool.html
and file://{$HOME}/rustup/toolchains/nightly-x86_64-unknown-linux-gnu/share/doc/rust/html/std/index.html
share the same theme
but every module does not, it's on a per-folder basis.

@workingjubilee
Copy link
Member

workingjubilee commented Sep 23, 2020

Unfortunately this means the only "automatically" "correct" solution I know of that makes this work is to rearchitect the folder layout of the std API docs into a mostly flat one, which would require extensive changes to rustdoc's manner of constructing directories, I believe. TRPL is fine at preserving themes precisely because it all is "flat", relatively speaking. Every other solution is at least some level of Dirty Hack. (like, say, providing a web extension which fixes this could be done, but...)

@jyn514
Copy link
Member

jyn514 commented Sep 23, 2020

Unfortunately this means the only "automatically" "correct" solution I know of that makes this work is to rearchitect the folder layout of the std API docs into a mostly flat one, which would require extensive changes to rustdoc's manner of constructing directories.

This seems like it could easily make doc/ directories many thousands of pages, which seems ... not ideal, especially for larger crates.

@ijackson
Copy link
Contributor Author

ijackson commented Sep 23, 2020 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-rustdoc-themes Area: Themes for HTML pages generated by rustdoc C-feature-request Category: A feature request, i.e: not implemented / a PR. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants