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

Valid conditions: false positive #888

Closed
michaelryanpeter opened this issue Sep 24, 2024 · 4 comments · Fixed by #891
Closed

Valid conditions: false positive #888

michaelryanpeter opened this issue Sep 24, 2024 · 4 comments · Fixed by #891

Comments

@michaelryanpeter
Copy link
Collaborator

Describe the bug
Currently, the valid conditions rule prints an error for the valid use case of using an inline conditional. Here is a link to the error: openshift/openshift-docs#82304 (comment)

Examples:

  • This word or expression triggers an illegitimate vale alert:

ifndef::olmv1-pullsecret-proc[For more information, see "Creating a pull secret for catalogs hosted on a secure registry".]

I am not sure if the regex can be extended to check for this use case. This might just be a false positive that we need to accept, since having a rule that catches open conditions seems more important than not flagging an error on this edge case.

@michaelryanpeter michaelryanpeter changed the title Valid conditions Valid conditions: false positive Sep 24, 2024
@aireilly
Copy link
Member

hmm. this sounds tricky! I'm not sure how we'd handle this tbh

@michaelryanpeter
Copy link
Collaborator Author

It might not be worth doing, tbh. I just thought I would point it out.

@aireilly
Copy link
Member

aireilly commented Sep 30, 2024

It's a dilly of a pickle. The one-line ifdef has the same syntax as a regular ifdef with closing statement. That's an unfortunate design when you are using regex matching instead of proper syntax validation.

Actually this is easily addressed with better regex

@aireilly
Copy link
Member

@michaelryanpeter vale sync to pick up the changes. LMK if you see anything else or this doesn't work for you

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 a pull request may close this issue.

2 participants