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

fix(linter): false positive #4197

Merged
merged 1 commit into from
Oct 7, 2024
Merged

fix(linter): false positive #4197

merged 1 commit into from
Oct 7, 2024

Conversation

ematipico
Copy link
Member

Summary

Closes #4190

We were not using text_trimmed() to check the var text. Instead, we were using text.

Test Plan

Added a new test


cc @Conaclos I moved a changelog paragraph. It was incorrectly listed as a linter fix, but it was a parser fix.

@ematipico ematipico requested review from a team October 7, 2024 08:10
@github-actions github-actions bot added A-Linter Area: linter L-CSS Language: CSS A-Changelog Area: changelog labels Oct 7, 2024
@Conaclos
Copy link
Member

Conaclos commented Oct 7, 2024

Using text instead of text_trimmed seems to be a recurring bug.
I wonder if we should not rename text to text_with_trivia and text_trimmed to text.

@ematipico
Copy link
Member Author

ematipico commented Oct 7, 2024

Using text instead of text_trimmed seems to be a recurring bug. I wonder if we should not rename text to text_with_trivia and text_trimmed to text.

text_with_trivia or text_not_trimmed are sound options. Good call. Howerver consider that we have many APIs that use the *trimmed suffix

@ematipico ematipico merged commit d5655e9 into main Oct 7, 2024
11 checks passed
@ematipico ematipico deleted the fix/false-positive-css-rule branch October 7, 2024 08:26
@Conaclos
Copy link
Member

Conaclos commented Oct 7, 2024

text_with_trivia or text_not_trimmed are sound options. Good call.

If we keep text_trimmed, text_not_trimmed or even text_untrimmed could be better than text_with_trivia.

Howerver consider that we have many APIs that use the *trimmed suffix

Good point. Perhaps it is preferable to keep this name.

Copy link

codspeed-hq bot commented Oct 7, 2024

CodSpeed Performance Report

Merging #4197 will not alter performance

Comparing fix/false-positive-css-rule (80e183b) with main (0d8cc8f)

Summary

✅ 105 untouched benchmarks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Changelog Area: changelog A-Linter Area: linter L-CSS Language: CSS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

💅 lint/nursery/noMissingVarFunction doesnt recognize var() after line break
2 participants