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

Convert .md links in rule documentation to full URLs #15904

Merged
merged 1 commit into from
Feb 3, 2025

Conversation

InSyncWithFoo
Copy link
Contributor

Summary

Related to these comments at #15862 and #15889.

Markdown links of the following format are an anti-pattern:

[`rule-name`](rule-name.md)

Only Mkdocs can resolve such links. When rendered by a client, typically an editor, they lead to nowhere. This might cause some frustration. Additionally, documentation for other rules (e.g., D211 and S606) uses the full URL form.

All such instances have thus been converted to full URLs:

[`rule-name`][A123]

[A123]: https://docs.astral.sh/ruff/rules/rule-name/

Test Plan

None.

Copy link
Contributor

github-actions bot commented Feb 3, 2025

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

@AlexWaygood AlexWaygood added the internal An internal refactor or improvement label Feb 3, 2025
Copy link
Member

@AlexWaygood AlexWaygood left a comment

Choose a reason for hiding this comment

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

Only Mkdocs can resolve such links. When rendered by a client, typically an editor, they lead to nowhere. This might cause some frustration.

I generally view the doc-comments for the diagnostic structs themselves as external, user-facing documentation rather than internal, developer-facing documentation. So for these doc-comments, it's much more important to me how they're rendered on our website than it is how they're rendered in a tooltip by an editor. (This is the opposite from most other doc-comments we have in the codebase, which are generally just for internal documentation.) We also have lots of other special links in these doc-comments that are understood by mkdocs but that I doubt will be rendered well in an editor tooltip -- e.g. [`preview`] will link to https://docs.astral.sh/ruff/preview/ even if [preview]: https://docs.astral.sh/ruff/preview/ is not at the bottom of the diagnostic struct's doc-comment.

However, I don't think these changes do any harm (other than maybe hardcoding our website domain in a few more places). I'm okay with them if others are.

@InSyncWithFoo
Copy link
Contributor Author

I generally view the doc-comments for the diagnostic structs themselves as external, user-facing documentation rather than internal, developer-facing documentation.

It is precisely due to them being user-facing that the links must be made accessible. ruff rule exists, and, as someone who uses the docs very often (IDE popups, command line, even rendered documentation view in RustRover), I want a better experience.

@MichaReiser
Copy link
Member

I sort of agree with @AlexWaygood but I also see @InSyncWithFoo's point.

My preferred solution would be to move the link generation to the mkdocs script. E.g., could the script automatically detect rule codes in [] and replace them with the full URL, similar to what we already do with options?

I'm okay landing this as a separate PR

@MichaReiser MichaReiser merged commit dfe1b84 into astral-sh:main Feb 3, 2025
21 checks passed
@InSyncWithFoo InSyncWithFoo deleted the rule-docs-links branch February 3, 2025 14:33
@AlexWaygood
Copy link
Member

Yeah, I admit I hadn't considered what the output of ruff rule would look like. Ideally I feel like we'd have "clean" doc-comments with links generated by mkdocs, but that we'd also "fixup" the doc-comment before printing it to the terminal in the ruff rule output. Then links like the ones that mkdocs generates for [`preview`] would also be clickable from the terminal if you ran ruff rule. This might be an unreasonable amount of complexity to add to ruff rule, though

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

Successfully merging this pull request may close these issues.

3 participants