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

Implement automatic selector if plugins are used #20

Merged
merged 5 commits into from
Mar 9, 2020

Conversation

kaste
Copy link
Contributor

@kaste kaste commented Nov 10, 2019

Fixes #16

The strategy here is to use a wide selector in the beginning and
then narrow it down during actual linting.

SublimeLinter needs a good selector as a fast check to match
views and installed linters because this first check runs on a shared
worker. T.i. we cannot just select all views and then decide later.

After the first check, it is always possible to abort loud or silently.
We do here silently because it's not a configuration error coming
from the user but an automatic algorithm bailing out.

With this implementation there comes the drawback that we have to flip
the previous default behavior. The default behavior was for the linter
to run on parts of the buffer marked as source.js concurrently. E.g.
it did run on the <script> tags when viewing a HTML file.

However, this default often did not produce good results. I.e. in an
HTML file it has no notion of gloabls introduced in one script which
are accessible in another script.

If the user wants this behavior, she now has to opt in by setting the
old selector 'source.js - meta.attribute-with-value' in the settings
on her own.

Implementation risks:

  • We deep import read_json_file. It is possible that this import can
    break since deep imports are not protected by any deprecation policy.

  • We have two side-effects: self.notify_unassign() followed by
    raise PermanentError(). It is possible that in the future we need
    to signal the same outcome differently.


IssueHunt Summary

Referenced issues

This pull request has been submitted to:


IssueHunt has been backed by the following sponsors. Become a sponsor

Fixes xojs#16

The strategy here is to use a wide `selector` in the beginning and
then narrow it down during actual linting.

SublimeLinter needs a good `selector` as a **fast** check to match
views and installed linters because this first check runs on a shared
worker. T.i. we cannot just select *all* views and then decide later.

After the first check, it is always possible to abort loud or silently.
We do here silently because it's not a configuration error coming
from the user but an automatic algorithm bailing out.

With this implementation there comes the drawback that we have to flip
the previous default behavior. The default behavior was for the linter
to run on parts of the buffer marked as `source.js` concurrently. E.g.
it did run on the `<script>` tags when viewing a HTML file.

However, this default often did not produce good results. I.e. in an
HTML file it has no notion of gloabls introduced in one script which
are accessible in another script.

If the user wants this behavior, she now has to opt in by setting the
old selector `'source.js - meta.attribute-with-value'` in the settings
on her own.

Implementation risks:

- We deep import `read_json_file`. It is possible that this import can
  break since deep imports are not protected by any deprecation policy.

- We have two side-effects: `self.notify_unassign()` followed by
  `raise PermanentError()`. It is possible that in the future we need
  to signal the same outcome differently.
linter.py Outdated Show resolved Hide resolved
@kaste
Copy link
Contributor Author

kaste commented Jan 7, 2020

Er, ping?

@sindresorhus
Copy link
Member

Sorry for the slow response. I've just had too many PRs to review.

@sindresorhus
Copy link
Member

sindresorhus commented Feb 22, 2020

With this implementation there comes the drawback that we have to flip
the previous default behavior. The default behavior was for the linter
to run on parts of the buffer marked as source.js concurrently. E.g.
it did run on the <script> tags when viewing a HTML file.

I'm fine with that. You should generally have minimal of inline JS, as a best practise.

linter.py Outdated Show resolved Hide resolved
linter.py Show resolved Hide resolved
linter.py Outdated Show resolved Hide resolved
@sindresorhus
Copy link
Member

Can you document the behavior in the readme?

@kaste
Copy link
Contributor Author

kaste commented Feb 28, 2020

LGTM!

@sindresorhus sindresorhus merged commit c6a66c7 into xojs:master Mar 9, 2020
@sindresorhus
Copy link
Member

Looks good. Thanks for working on this. 👍

@kaste kaste deleted the automatic-selector branch August 2, 2022 11:49
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.

Automatically configure alternative syntax scopes when available
2 participants