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

Add more clickable struct names in comments #1419

Merged
merged 10 commits into from Apr 3, 2022
Merged

Conversation

4JX
Copy link
Contributor

@4JX 4JX commented Mar 25, 2022

Replaced all instances found through the following regex:

([^[])(`(?!Arc)(?!Rc)(?!PartialEq)(?!Clone)(?!Some)(?!None)(?!MacOS)(?!EasyMark)(?!Err)(?!Home)(?!End)(?!Trackpad)[A-Z][a-z]{1}\w*`)

Tried to keep it conservative as to not make false positives, haven't found any wrong replacements myself.

Example of a clickable struct name:

4JX added 2 commits March 25, 2022 22:47
This allows them to be clicked when viewed through a code editor
@4JX 4JX changed the title Clickable struct names in comments Add more clickable struct names in comments Mar 25, 2022
@emilk
Copy link
Owner

emilk commented Mar 25, 2022

Thank you for this!

Unfortunately it is not enough to just enclose in [], they also need to be in scope, or have a qualified path to them.

You can see the problem by running

cargo doc -p emath -p epaint -p egui -p eframe -p epi -p egui_web -p egui-winit -p egui_glium -p egui_glow --lib --no-deps --all-features
…
warning: unresolved link to `PathShapes`
  --> epaint/src/bezier.rs:57:56
   |
57 |     /// Convert the cubic Bézier curve to one or two [`PathShapes`].
   |                                                        ^^^^^^^^^^ no item named `PathShapes` in scope
   |
   = note: `#[warn(rustdoc::broken_intra_doc_links)]` on by default
   = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]`

(unfortunately I haven't figured out how to get cargo doc to fail on warnings to that the CI can catch this)

@4JX
Copy link
Contributor Author

4JX commented Mar 26, 2022

Thank you for this!

Unfortunately it is not enough to just enclose in [], they also need to be in scope, or have a qualified path to them.

Whoops, I assumed they were working as VSCode did correctly place links on them. Will fix it.

@4JX 4JX marked this pull request as draft March 26, 2022 00:10
@Mingun
Copy link
Contributor

Mingun commented Mar 26, 2022

(unfortunately I haven't figured out how to get cargo doc to fail on warnings to that the CI can catch this)

Maybe adding #![deny(rustdoc::broken_intra_doc_links)] at the crate root solve this?

egui/src/data/input.rs Outdated Show resolved Hide resolved
epaint/src/color.rs Outdated Show resolved Hide resolved
@4JX 4JX marked this pull request as ready for review April 1, 2022 21:39
@4JX
Copy link
Contributor Author

4JX commented Apr 1, 2022

One fight with github's merging later this should be ready for merging with no more broken links.

@4JX
Copy link
Contributor Author

4JX commented Apr 1, 2022

Spoke too soon, that one does not show on my machine but it does on CI. Maybe caused by #1125 stopping the build from completing.

@emilk
Copy link
Owner

emilk commented Apr 3, 2022

Wow, you are amazing for doing all this work! There are still 12 broken links with cargo doc --document-private-items --no-deps --all-features though:

warning: unresolved link to `Ui`
  --> egui/src/layout.rs:10:43
   |
10 |     /// This is the minimal size of the [`Ui`].
   |                                           ^^ no item named `Ui` in scope
   |
   = note: `#[warn(rustdoc::broken_intra_doc_links)]` on by default
   = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]`
…

@emilk emilk merged commit 6091370 into emilk:master Apr 3, 2022
@emilk
Copy link
Owner

emilk commented Apr 3, 2022

Thanks ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants