Skip to content

typos improvements #583

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

typos improvements #583

wants to merge 3 commits into from

Conversation

bmrips
Copy link
Contributor

@bmrips bmrips commented Apr 13, 2025

This PR comprises three changes to the typos hook:

  • Use structured configuration by making typos.settings.configuration an attrs. This change is not backwards compatible.
  • Add the typos.settings.force-exclude option that maps to typos' --force-exclude argument. In addition, enable it by default since you expect exclude globs set via the --exclude CLI option or the configuration file's [files.extend-exclude] option to be effective.
  • Enable multiple exclusion patterns in typos.settings.exclude.

@joscha
Copy link
Contributor

joscha commented May 6, 2025

  • Add the typos.settings.force-exclude option that maps to typos' --force-exclude argument. In addition, enable it by default since you expect exclude globs set via the --exclude CLI option or the configuration file's [files.extend-exclude] option to be effective.

🎉 , I was wondering why this didn't behave as expected

@bmrips bmrips force-pushed the typos-improvements branch from 4b5fce7 to b635a9e Compare May 6, 2025 14:59
@srid
Copy link
Contributor

srid commented May 31, 2025

I find a strange behaviour when using this PR.

For this setting,

          settings.exclude = [
            "*.nix"
            ".hlint.yaml"
          ];

it generates a YAML with the following CLI:

❯ rg bin/typos .pre-commit-config.yaml
101:          "entry": "/nix/store/n97528zsiq2yshvfq5whl4xpbydlnddl-typos-1.32.0/bin/typos --config /nix/store/0my2fa9n84gy2jk0wlclww22xzwckw0l-typos-config.toml --exclude *.nix  --force-exclude",

Note that --exclude *.nix part. Somehow only the first exclude entry (*.nix) is being used, the rest (.hlint.yaml) is ignored.

But I'd think that the following use of map should have included all entries:

excludeFlags = lib.map (glob: "--exclude ${glob}") exclude;

Is this something to do with the use of coercedTo?

@bmrips bmrips force-pushed the typos-improvements branch from b635a9e to 6007ee9 Compare May 31, 2025 22:49
@bmrips
Copy link
Contributor Author

bmrips commented May 31, 2025

Note that --exclude *.nix part. Somehow only the first exclude entry (*.nix) is being used, the rest (.hlint.yaml) is ignored.

Thank you for the notification! It turned out that mkCmdArgs only considers the first and second items of the list, such that following items are ignored. Should be fixed now.

srid added a commit to juspay/vira that referenced this pull request Jun 1, 2025
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.

3 participants