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

Feature: Textrules #247

Merged
merged 20 commits into from
Jun 9, 2023
Merged

Feature: Textrules #247

merged 20 commits into from
Jun 9, 2023

Conversation

DaveMcEwan
Copy link
Contributor

@DaveMcEwan DaveMcEwan commented May 22, 2023

Implement textrules feature.

  • This is a new class of rules which are applied before any preprocessor or syntax parsing. Feature: Extend rules mechanism to further levels of abstraction (long term). #237
  • The new rules are placed into src/textrules/.
  • The previous/existing rules all operate on a concrete syntax tree, so these are moved from
    src/rules/ to `src/syntaxrules/. Dev Question: New class of rules, textrules, working on lines instead of syntax. #244
  • As far as possible, conventions for rules/syntaxrules are followed:
    • Unittests in testcases, build.rs, and src/main.rs.
    • Generation of MANUAL in md and build.rs.
  • Only 4 example textrules are included in this PR to keep the diff manageable.
    Other rules which could be written later:
    • Check that define macro names fit required/forbidden regex.
    • ...
  • While implementing textrules/style_textwidth, I noticed a bug in the printer where multi-byte characters in a violating line would cause the hint and reason to be indented too far.
    This PR includes a fix (a9def1b).
  • The behaviour of --example and --update has been validated locally, but there are no unittest for this (yet).

To see the diff for just moving "rules" to "syntaxrules", I've made an intermediate branch:
#246
Note, I forgot to include the changes to md/ruleset-*.md and rulesets/* in this intermediate branch, sorry.

To see the diff between that branch and this one, i.e. the real changes:
https://github.com/DaveMcEwan/svlint/compare/textrules-move...DaveMcEwan:svlint:textrules?expand=1

- Passes `cargo test`.
- Found bug in printer when violating lines contain multi-byte characters.
- Missing updates to MANUAL.
- PDF tested with `make MANUAL-dev`.
The appearance of "Fail" can be confusing.
Only used in development/unittests, not in normal runtime.
Now, all of the rulesets are fully implemented in rules, not with grep.
@dalance
Copy link
Owner

dalance commented Jun 9, 2023

Thank you for your great work!
I'll merge this.

@dalance dalance merged commit 6ac8f35 into dalance:master Jun 9, 2023
This was referenced Jun 9, 2023
@DaveMcEwan DaveMcEwan deleted the textrules branch June 19, 2023 08:54
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.

2 participants