Skip to content

feat: flag bad descriptive text#190

Merged
SecondSkoll merged 6 commits into
mainfrom
DOCPR-2145/flag-bad-descriptive-text
Apr 14, 2026
Merged

feat: flag bad descriptive text#190
SecondSkoll merged 6 commits into
mainfrom
DOCPR-2145/flag-bad-descriptive-text

Conversation

@SecondSkoll
Copy link
Copy Markdown
Contributor

Addresses #145

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds a new Vale (Canonical) style rule to flag non-descriptive link text (e.g., “here”, “click here”) to improve accessibility, addressing issue #145.

Changes:

  • Enable Canonical.027-bad-descriptive-links in the Vale configuration.
  • Add a new Vale rule (027-bad-descriptive-links) with regexes covering Markdown, reST, and Sphinx/MyST roles.
  • Add manifest-driven pytest cases to validate the new rule behavior for md/rst.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
vale.ini Registers the new Canonical rule and sets its configured alert level.
styles/Canonical/027-bad-descriptive-links.yml Implements the new rule patterns and message.
tests/data/manifest.yml Adds test coverage for the new rule (plus relocates the existing 026 block).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread vale.ini
Comment thread styles/Canonical/027-bad-descriptive-links.yml Outdated
Comment thread styles/Canonical/027-non-descriptive-link-text.yml
Comment thread styles/Canonical/027-non-descriptive-link-text.yml Outdated
@AnneCYH
Copy link
Copy Markdown
Contributor

AnneCYH commented Apr 9, 2026

Similar feedback to @dwilding (good catch!)
Accounting for intersphinx format will be good.
But nice rule!

@AnneCYH AnneCYH self-requested a review April 9, 2026 13:49
AnneCYH
AnneCYH previously approved these changes Apr 9, 2026
Copy link
Copy Markdown
Contributor

@AnneCYH AnneCYH left a comment

Choose a reason for hiding this comment

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

Approve with the expectation of addressing David's feedback.
Nice rule!

Comment thread tests/data/manifest.yml
- id: bad-link-text-md
filetypes: [md]
content: |
See [here](https://example.com) for details.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Food for thought: I feel like the boilerplate should be self-explanatory, but with these full sentences it could look like we're testing for specific verbs and phrases. Might be clearer to leave out the surrounding text.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The idea is that the test cases aren't just atomic representations of cases to capture - but rather a semi-realistic case that should be captured (or not captured) to give some level of confidence that it's ready to be applied. This has been important for rules like 004-Canonical-product-names.

Comment thread vale.ini
Canonical.025b-latinisms-to-reconsider = suggestion
Canonical.025c-latinisms-to-avoid = suggestion
Canonical.026-hints-tips = suggestion
Canonical.027-bad-descriptive-links = warning
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What was the motivation for making this one a warning?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It was the level Daniele wanted, but also given possible issues with accuracy, completeness, or even some possible false positives it's probably good to have at this level.

The idea will be for error level results to block PRs - not warnings or suggestions.

@SecondSkoll SecondSkoll requested a review from dwilding April 13, 2026 03:49
@SecondSkoll SecondSkoll merged commit 8741057 into main Apr 14, 2026
4 checks passed
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.

5 participants