-
Notifications
You must be signed in to change notification settings - Fork 13.5k
Refactor intra doc link code #76684
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
Refactor intra doc link code #76684
Conversation
Previously, `resolve` would immediately check that `module_id` was non-empty and give an error if not. This had two downsides: - It introduced `Option`s everywhere, even if the calling function knew it had a valid module, and - It checked the module on each namespace, which is unnecessary: it only needed to be checked once. This makes the caller responsible for checking the module exists, making the code a lot simpler.
Some changes occurred in intra-doc-links. cc @jyn514 |
The number of parameters is kind of awful ... I might add a struct that groups them into one. |
Yeah no this had way too many compile errors for me to deal with right now, if I do add it will be in a follow-up PR |
} | ||
} | ||
} | ||
match self.resolve_with_disambiguator( |
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.
This is just ?
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.
This returns ()
, not Option
. So it can't use ?
.
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.
lgtm
@bors r=manishearth Note to anyone making rollups: this is unlikely to fail any tests, but is extremely likely to conflict with other intra-doc link PRs. |
📌 Commit 8a13fc4 has been approved by |
☀️ Test successful - checks-actions, checks-azure |
I got tired of
fold_item
being 500 lines long.This is best reviewed one commit at a time with whitespace changes hidden.
There are no logic changes other than the last commit making a parameter checked by the caller instead of the callee.
r? @Manishearth