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

Content masked by patterns.txt shouldn't count to minified ratio #53

Closed
1 of 3 tasks
Piedone opened this issue May 26, 2023 · 9 comments
Closed
1 of 3 tasks

Content masked by patterns.txt shouldn't count to minified ratio #53

Piedone opened this issue May 26, 2023 · 9 comments
Milestone

Comments

@Piedone
Copy link

Piedone commented May 26, 2023

This Markdown file, a Readme, has a long line at the top (containing NuGet shields; this needs to be one line as enforced by MD linting). check-spelling (v0.0.21) ignores this file with the following warning:

Skipping src/Modules/Lombiq.Hosting.Tenants/Readme.md because average line width (1047.33333333333) exceeds the threshold (1000). (minified-file)

However, it's not a minified file and should rather be checked.

So, I'd suggest one or more of the following options:

  • If a line is ignored by patterns.txt, then that ignore also takes effect for this modification check. I.e. if I ignore the long line, the rest of the file should be checked still.
  • Exclude .md files from the minification check.
  • Provide a more granular configuration instead of just blanket disable_checks: minified-file, like a pattern for file extensions to be excluded from this check.
@jsoref
Copy link
Member

jsoref commented May 28, 2023

Oh, factoring in patterns.txt seems quite reasonable. It isn't a huge amount of overhead, just appending after-filtered line lengths and using that instead of a tell call.

I like that idea.

Implemented in 4dabda0 (in prerelease).

@jsoref jsoref added this to the v0.0.22 milestone May 28, 2023
@Piedone
Copy link
Author

Piedone commented May 29, 2023

Thank you for being so quick to respond. I don't really see how the linked commit implements this though, could you explain?

@jsoref
Copy link
Member

jsoref commented May 29, 2023

When things are hit by patterns, they're currently masked (with =) instead of elided. This enables proper offset reporting for found items. The last step where it calculates the ratio used to take the current file position (using tell) and line number to calculate a ratio. With the change, it removes the runs of 3 =s which are presumed to not be minified content but are most likely pattern replacements and then sums the remaining line length to calculate the amount of content seen. (It will technically treat a really long line of =s that aren't a pattern replacement as nothing as well, but I don't think that's something that generally happens in minified content, so I'm ok with that false negative.)

Could you try using 2d27459 and see if it works?

@Piedone
Copy link
Author

Piedone commented May 29, 2023

Ah OK, my whole misunderstanding stems from not seeing that by "patterns" you actually meant patterns.txt, not something like a separate configuration with an ignore pattern for disabling the minification check. All clear now.

I tested it out, and it works, no more warning: https://github.com/Lombiq/Open-Source-Orchard-Core-Extensions/actions/runs/5114328703 Spell-checking otherwise in the same file still works as it should: https://github.com/Lombiq/Open-Source-Orchard-Core-Extensions/actions/runs/5114372557

So, thank you!

@jsoref jsoref changed the title Don't consider Markdown files minified Content masked by patterns.txt shouldn't count to minified ratio May 29, 2023
@Piedone
Copy link
Author

Piedone commented Sep 18, 2023

Whe do you intend to release v0.0.22?

@jsoref
Copy link
Member

jsoref commented Sep 18, 2023

I'm hoping this week. I think I've done all of my code changes and am slowly beating the documentation into shape.

Can you give the current https://github.com/check-spelling/check-spelling/tree/prerelease a kick for a day?

@Piedone
Copy link
Author

Piedone commented Sep 18, 2023

I tested it out and worked great, thanks. See https://github.com/Lombiq/GitHub-Actions/actions/runs/6227964134/job/16903732401?pr=234 and https://github.com/Lombiq/Open-Source-Orchard-Core-Extensions/actions/runs/6228059558/job/16904012854. I needed to fix an ignored-expect-variant error before that, here, but that's good.

@jsoref
Copy link
Member

jsoref commented Sep 29, 2023

@jsoref jsoref closed this as completed Sep 29, 2023
@Piedone
Copy link
Author

Piedone commented Sep 29, 2023

Awesome, thank you! Updated our GitHub Actions project: Lombiq/GitHub-Actions#234

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

No branches or pull requests

2 participants