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

IndexItem should be serialised as an object rather than an array #34678

Closed
nrc opened this issue Jul 6, 2016 · 5 comments
Closed

IndexItem should be serialised as an object rather than an array #34678

nrc opened this issue Jul 6, 2016 · 5 comments
Labels
E-help-wanted Call for participation: Help is requested to fix this issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@nrc
Copy link
Member

nrc commented Jul 6, 2016

See html/render.rs for the Rust side and main.js and possibly other places for the JS side.

I see no reason why this struct is serialised as an array, thus losing the information about which field is which. Using an object would retain that information and make things less error-prone, I believe.

@nrc nrc added T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. E-help-wanted Call for participation: Help is requested to fix this issue. labels Jul 6, 2016
@GuillaumeGomez
Copy link
Member

You've got this issue when running rustdoc on librustdoc? Can you give me the rustdoc command you used to generate doc please?

@nrc
Copy link
Member Author

nrc commented Jul 6, 2016

This isn't a bug, but a suggestion for improvement. It's just a refactoring of the code, it doesn't need to be run on anything.

Having read the code a bit more, I think the reason it might be done like this is for efficiency, but I don't think that is important here - doesn't really matter if we use a few more bytes for the index, right?

@GuillaumeGomez
Copy link
Member

Ah ok, my bad. I thought the output of this struct in particular was bad. Sorry about the misunderstanding! 😺

@jonas-schievink
Copy link
Contributor

Seems like this was done deliberately in #13431

@nrc
Copy link
Member Author

nrc commented Jul 19, 2016

Based on #13431, looks like we should not do this.

@nrc nrc closed this as completed Jul 19, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
E-help-wanted Call for participation: Help is requested to fix this issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

3 participants