-
Notifications
You must be signed in to change notification settings - Fork 13.2k
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-json: Rename Path::name
to path
, and give it the path again.
#135799
Conversation
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for putting this together! 🙏
tests/rustdoc-json/path_name.rs
Outdated
//@ is "$.index[*][?(@.name=='U3')].inner.type_alias.type.resolved_path.name" '"pub_mod::InPubMod"' | ||
pub type U3 = InPubMod3; | ||
|
||
// Check we only have paths for structs at there origonal path |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// Check we only have paths for structs at there origonal path | |
// Check we only have paths for structs at their original path |
Thanks for this PR! A revert was necessary because we need to prevent #134880 from being included in the beta, as it would break tools and we cannot change |
N.B: Rustdoc-Json is currently unstable, and not usable on beta/stable. The purpose of the revert is to fix nightly.
I’m not sure i follow. |
I don't follow the last part either— |
Sorry, I wasn't clear. What I meant was For the code: #![no_std]
pub type Foo = core::str::SplitAsciiWhitespace<'static>; The json output will be {
"index": {
"0": {
"id": 0,
"inner": {
"type_alias": {
"generics": {"params": [], "where_predicates": []},
"type": {
"resolved_path": {
"args": {"angle_bracketed": {"args": [{"lifetime": "'static"}], "constraints": []}},
"id": 1,
"name": "SplitAsciiWhitespace"
}
}
}
},
"name": "Foo"
},
"2": {
"id": 2,
"inner": {"module": {"is_crate": true, "is_stripped": false, "items": [0]}},
"name": "mcve"
}
},
"paths": {
"0": {"crate_id": 0, "kind": "type_alias", "path": ["mcve", "Foo"]},
"1": {"crate_id": 1, "kind": "struct", "path": ["core", "str", "iter", "SplitAsciiWhitespace"]}
},
"root": 2
} Here, I hope I made it clear but if not please ask me, I might be wrong so please let me know. |
Correct, both of them built of
For source URL's, rustdoc-html goes through the The issue is that |
A though while doing the docs: Should this field be renamed to not |
rustdoc-json-types is a public (although nightly-only) API. If possible, consider changing |
Path::name
have the full name again.Path::name
to path
, and give it the path again.
@GuillaumeGomez the code can be reviewed as-is, it's ready. But don't merge it yet, I want to squash commits before that. (But don't want to yet, incase we decide to not do the field rename, as it'd be easier to revert). |
This comment has been minimized.
This comment has been minimized.
It'd definitely be a better match for what it currently contains. It works for me. |
src/rustdoc-json-types/lib.rs
Outdated
/// This will be the path where the type is *used*, so it may be a | ||
/// re-export. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion, feel free to take it or leave it: I'm not 100% sure what "used" means in this context. Is it the path referenced in the code being documented, either directly or via an import? AKA "what path would we have to remove to break the code being documented"
Perhaps an example could help?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added an example:
pub type Vec1 = std::vec::Vec<i32>; // path: "std::vec::Vec"
pub type Vec2 = Vec<i32>; // path: "Vec"
pub type Vec3 = std::prelude::v1::Vec<i32>; // path: "std::prelude::v1::Vec"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!
Closes rust-lang#135600 Effectivly reverts rust-lang#134880
707ef52
to
9c0e32b
Compare
Docs improved. Commits squashed. |
Looks good to me, thank you! Seems ready to merge, if I'm not mistaken? I'm eager to use it in cargo-semver-checks ASAP, so that we narrow the window of nightlies with which we're incompatible :) |
I already approve so it's up to @aDotInTheVoid whenever they want. :) |
@bors r=GuillaumeGomez rollup (Sorry, I didn't realize github's green check meant r=you here. I don't think there's a norm here) |
Ah sorry, miscommunication I think. You said you wanted my approval but not my merge because you wanted to make another small change. So I approved and for me it was all done. Should have precised. ^^' |
Closes: #135600.
Reverts #134880 (Effectively, but not actually, as the
FORMAT_VERSION
needs to be bumped, changed docs/tests). CC @AS1100K.Also CC @obi1kenobi @LukeMathWalker
Still needs before being merge-ready:
Path::name
topath
, and give it the path again. #135799 (comment))r? @GuillaumeGomez