Shopify / theme-check

The Ultimate Shopify Theme Linter
Other
337 stars 95 forks source link

Feature Request - Add flag to filter errors for specific files #499

Closed sdn90 closed 1 month ago

sdn90 commented 2 years ago

I think this is a pretty important feature for people who are adding Theme Check to existing themes. The codebase I'm working on has a huge amount of errors and it's not realistic for us to fix everything in one pass.

My ideal situation would be: shopify theme check [file1] [file2]

I think this should work with lint-staged without having to configure too much stuff

charlespwd commented 2 years ago

This could work but we're faced with a choice on the implementation side:

  1. We disable whole theme checks -- checks that work on multiple files at once -- when going that route (e.g. unused snippets, matching translations)
  2. We keep whole theme checks

What would you expect the tool did for you if you didn't look at the docs?

eloyesp commented 2 years ago

I would expect the whole theme checks to be Enabled, but only report errors for the given files. And if autofix is enabled, then autofix should not touch any file out of the args.

charlespwd commented 2 years ago

I've added partial support for this in Shopify/theme-check-action. Now that we annotate files, if you have the following configuration, only the diff'ed files will be annotated on your PR:

      - name: Theme Check
        uses: shopify/theme-check-action@v1
        with:
          theme_root: '.' # optional, could be './dist'
          token: ${{ github.token }}
          base: main

Is that enough for you?

The approach I used was to use -o json option and filter the results after the fact. I didn't go with the CLI argument route because shells typically have a limit of arguments you pass to them. By outputting the list of diffed files to a file, we can avoid the 250 argument limit as well as requiring you to write a complicated pattern that might work.

A --filter might still be desirable. But I'm not sure it's that great for this particular use case. Maybe if we let that accept files as input. I'm open to suggestions.

lukeh-shopify commented 1 month ago

👋🏻 Hi @sdn90! Thanks for reporting this. We are deprecating the Ruby version of theme check as it is no longer receiving developer support. If you're still having issues with the suggestion by @charlespwd to use the latest version of the Github action, then please open a new issue there. Thanks!