Skip to content

Restore colour to rustdoc #17226

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

Merged
merged 2 commits into from
Sep 17, 2014
Merged

Restore colour to rustdoc #17226

merged 2 commits into from
Sep 17, 2014

Conversation

ftxqxd
Copy link
Contributor

@ftxqxd ftxqxd commented Sep 13, 2014

This not only gives back the colour to structs, traits, and functions, but also restores the long-lost colour to enums. I’ve run linkchecker and there don’t seem to be any more invalid links than usual.
I’m not entirely sure if the way I’ve implemented this is very idiomatic or reliable, but it seems to work.

Closes #17131.
Closes #16712.
Closes #16711.

} else {
clean::TypeTypedef
};
record_extern_fqn(cx, did, typ);
Copy link
Member

Choose a reason for hiding this comment

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

This may end up causing confusion as it doesn't take into account did.krate, which can be nonzero in the case of inlining documentation from other crates. This could sporadically cause typedefs to become enums or vice versa perhaps.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think you’re right: rustrt renders Option as a type instead of an enum (e.g., rustrt::args::clone).

@Gankra
Copy link
Contributor

Gankra commented Sep 13, 2014

@P1start Can you host a rust documentation instance running this change? We've had some unfortunate layout regressions recently, and I'd like to avoid that.

@ftxqxd
Copy link
Contributor Author

ftxqxd commented Sep 13, 2014

@gankro I’ve hackily set something up with the documentation generated by this.

@Gankra
Copy link
Contributor

Gankra commented Sep 13, 2014

@P1start Awesome, thanks! Documentation looks exactly as broken as always 👍

One thing I noticed is that generics seem weird as uncoloured now. Would it be possible to colour them?

@ftxqxd
Copy link
Contributor Author

ftxqxd commented Sep 13, 2014

I don’t think that colouring generics would be a good idea, as every other coloured thing is a link to another page. If anything, we should colour typedefs, primitives &c. something other than black to make them stand out more than things which aren’t documented (generics and private types).

@Gankra
Copy link
Contributor

Gankra commented Sep 13, 2014

That's a fair point. Perhaps generics could simply be coloured grey to make them visually distinct from keywords and sigils, but otherwise "dead"?

@ftxqxd ftxqxd force-pushed the rustdoc-colour branch 2 times, most recently from d44ebff to 778a343 Compare September 14, 2014 08:06
@ftxqxd
Copy link
Contributor Author

ftxqxd commented Sep 14, 2014

I’ve tried another path that seems to work much better. (The old one was still generating bad cross-crate links sometimes.) I added a bool to rustc::middle::def::Def’s DefTy to indicate whether or not it’s an enum.

@eddyb
Copy link
Member

eddyb commented Sep 14, 2014

Adding to Def is a definite no.
Have you tried using free functions in middle::ty to automatically handle the cross-crate case?
That is what many rustc passes rely on.

@ftxqxd
Copy link
Contributor Author

ftxqxd commented Sep 14, 2014

@eddyb Sorry, I don’t know which functions you’re talking about.

@ftxqxd
Copy link
Contributor Author

ftxqxd commented Sep 15, 2014

@alexcrichton I had to update the last commit because Travis was showing some legitimate errors.

This is done by adding a new field to the `DefTy` variant of `middle::def::Def`,
which also clarifies an error message in the process.

Closes rust-lang#16712.
alexcrichton added a commit to alexcrichton/rust that referenced this pull request Sep 17, 2014
@bors bors merged commit 8b88811 into rust-lang:master Sep 17, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants