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

Add support for "unsafe" corrections #427

Closed
charlespwd opened this issue Nov 29, 2021 · 1 comment
Closed

Add support for "unsafe" corrections #427

charlespwd opened this issue Nov 29, 2021 · 1 comment
Labels
area:theme-check enhancement New feature or request

Comments

@charlespwd
Copy link
Contributor

charlespwd commented Nov 29, 2021

Some of our corrections could be unsafe. Examples:

  • a script needs to be parser blocking, so defer or async breaks the site
  • the output of a | img_tag doesn't have its width and height CSS attributes specified. Switching to image_tag would break the site.
  • etc.

IMO if you run theme-check -a, the result of that operation should be safe and you should be confident that your site still works once applied. Same for if you do fix all auto-correctable problems from the editor.

Because of this, I'm very hesitant when it comes to corrections that would be considered unsafe (e.g. #434).

According to this part of the docs, it looks like we might be able to add some sort of confirmation popups for corrections such as these.

I'd be more comfortable shipping autocorrections that can be flagged "unsafe." So that we require confirmation from the user to apply them. Would open the door to more correction opportunities.

Maybe all we need is some kind of flag to add when we do add_offense ? Something like unsafe: true. Then we'd be able to do this:

  • If unsafe and editor supports change annotations, make a change annotation that requires confirmation. Otherwise don't offer the unsafe correction at all.
  • If unsafe, do not correct with theme-check -a
@charlespwd
Copy link
Contributor Author

We support suggestions / fixes in theme check 2.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:theme-check enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants