WIP: Next #666

braver · 2017-11-13T17:03:02Z

A PR to track a possible upcoming (pre)-release where we combine some new features with a bunch of cleaning up.

Most of the work done by @pykong at PR #665 (closed)

fixes #662, #661, #649, #639, #619, #618, #609, #274, #292, #299, #569, #663, #279, #348, #394, #457, #460, #461, #524, #557, #570, #452, #212, #115,

The number of this PR should tell you something 🤘

Open issues:

Some core issues we want to register:

~~inconsistent type when set vs get to settings~~

Feature requests:

Tooltip on hover over a highlight, displaying the errors for that specific position (which may be multiple, but it shouldn't show all errors for the line)
It should be possible to disable each popup.
Replace Report (Open Files) and Show All Errors with an output panel, like the one in LSP. Will fix list errors in an output panel #115 and Passive Warnings Setting Per Linter #212

We could consider dropping the new line report popup in favour of these two features, but it's ok to first try and see how it feels and then decide. I think reporting per line isn't all that natural, but perhaps that depends on the linters you're using.

jrappen · 2017-11-22T18:37:37Z

@groteworld Looking at eda3a8a there is still a Gratipay reference in the README at the bottom. Also you can register a paypal.me URL (and use it like paypal.me/groteworld/5usd), which I personally find to be easier to use. If you want to keep the recurring option for PayPal though, you'll need the button (if they didn't change that meanwhile for the paypal.me site, didn't check).

braver · 2017-11-22T18:39:42Z

@jrappen not sure this PR is the place for this discussion, but I'll remove it from the README too.

jrappen · 2017-11-22T18:46:03Z

@pykong Your changes from 8bc595a do not address *.hidden-color-scheme files, only *.sublime-color-scheme files. Compare the last paragraph from Jon's message here: https://forum.sublimetext.com/t/dev-build-3154/33286.

braver · 2017-11-22T18:50:21Z

I'm not sure what you mean. > 3148 We don't create color schemes at all anymore, instead we fully rely on scopes already available in a given color scheme (or the region.colorish scopes). < 3148 hidden tmTheme files are created.

FichteFoll · 2017-11-23T07:17:21Z

Glad to see changes happening to SublimeLinter. This opportunity should probably be used to solve the awkward situation of Python linters being evaluated within the built-in environment, which causes various issues and is generally quite hack with any version that is not 3.3. I don't think there is an issue for this yet, but it's been a while since I last looked for one. This has been on my list for a while now, but I won't be able to file a proper issue until Sunday.

braver · 2017-11-23T07:46:25Z

While definitely something is happening, we’re mosty attacking the UI. The entirety of SublimeLinter needs attention and the issue you point out is one of them. We could definitely use some help in that area, so if anyone wants to open a PR that’d be very welcome.

FichteFoll · 2017-11-23T10:56:58Z

I can imagine that help is always welcome, but I'm unable to provide it here currently. There are just too many things I'm dingbat already.

FichteFoll · 2017-11-23T11:02:30Z

*doing. Thanks autocorrect and thanks github for making editing comments on mobile a pain.

braver · 2017-11-23T11:14:28Z

Yes, dingbat is most likely to be the word you were looking for 😀

Just taking a peek at it and having some feedback is also good!

gerardroche · 2017-11-23T16:01:25Z

Seems to ok for me with just one issue:

Please add more granularity to the default style scopes. Specifically a namespace, but also add types to the scopes.

The reason is to allow color schemes to enhance support specifically for the plugin, because while a color scheme can define styles for the colorish regions it may want to do something specific for a plugin.

I've already talked elsewhere on this so I'm not going to rehash it all. I recommend postfixing a namespace so that color schemes can do this:

    <dict>
        <key>scope</key>
        <string>region.yellowish sublimelinter</string>
        <key>settings</key>
        <dict>
            <key>foreground</key>
            <string>#fd971fa6</string>
        </dict>
    </dict>

Appending it may cause potential issues later. Postfixing is more powerful.

Also add types but I suggest not coupling it to the namespace for the most flexibility:

    <dict>
        <key>scope</key>
        <string>region.yellowish warning.sublimelinter sublimelinter</string>
        <key>settings</key>
        <dict>
            <key>foreground</key>
            <string>#fd971fa6</string>
        </dict>
    </dict>

One other thing that would be nice is to differentiate between debug and verbose for plugin debug messages and lint warnings. For example when you enable debug mode you get messages like:

stdin:1:1: F401 'sublime' imported but unused
stdin:3:1: E302 expected 2 blank lines, found 1
stdin:7:1: F821 undefined name 'f'
stdin:8:1: W391 blank line at end of file

I often want to see those messages because I want to lookup the error code (there's probably several features in there). Enabling debug mode gives too much noise really all you want is verbose linting information.

PS. I've tested against Python and PHP (there's an open pr for 7.2 support).

--- edit ---

I suspect allowing the scope to be configurable is going to cause problems. I can't see a use case.

FichteFoll · 2017-11-23T16:42:02Z

Error codes are within the domain of plugins. SublimeLinter-flake8 has an option for this, for example (though imo it should be enabled by default).

Regarding more specific scopes, what about prefixing the sublimelinter scope?

braver · 2017-11-23T17:42:44Z

You’ll be able to view a list of errors per line but hovering over the gutter, and there are report options already in place. The console is not the place for such features, so we definitely won’t be enhancing the logging options there.

I need to check how it works, but if I’m right we’re already baking in a scope that will allow you to target the highlights from a color scheme. I don’t think you want users to have to “fight” against a color scheme just to get the colors they want. Region.colorish is already in hands of the color scheme, as are things like markup.error. A color scheme can advertise to the user which scopes are suitable for linter highlights, but we’re explicitly putting this in the hands of users. This will allow them to differentiate between different linters in a single file, which is one of the most frequently recurring feature requests. A color scheme cannot reasonably provision for such situations.

FichteFoll · 2017-11-27T16:50:46Z

messages/4.0.0-rc.1.txt

+The tools menu now only lists the clear_cache command.
+Commands are now named sublime_linter_* to comply with naming conventions.
+
+Commands to change settings or toggle linters have been removed.


What about the situation when you want to have a linter disabled by default but enable it for certain projects only?

Sure, nothing about the enabled/disabled settings has changed, just the command has been removed. You can enable linters in project settings or in sublimelinterrc files.

Edit: all the command did was enable/disable a linter globally. If you want to replicate this, you can simply use package control to disable/enable the specific linter plugin.

Right, didn't read this correctly. All good then.

pykong · 2017-11-27T22:32:17Z

Teaser...

darkred · 2017-11-28T21:36:11Z

Sorry for bothering you, but I'd like to try your work. I've been trying but without success.
Could you please tell me, what am I doing wrong?

STR:

Using ST3 3154 x64 (clean installation).
I download https://github.com/SublimeLinter/SublimeLinter3/archive/next.zip .
Inside the resulting SublimeLinter3-next.zip there's a folder SublimeLinter3-next with the content.
So, I copy it as SublimeLinter into C:\Users\Kostas\AppData\Roaming\Sublime Text 3\Packages

Then, as I run ST I get:

FileNotFoundError: [Errno 2] No such file or directory: 'C:\\Users\\Kostas\\AppData\\Roaming\\Sublime Text 3\\Packages\\User\\SublimeLinter.sublime-settings'`

I open: Preferences | Package Settings | SublimeLinter -> Settings
and I copy-paste the settings from the left (Default settings) to the right (USer).
Then, as I restart ST, I get this:

Traceback (most recent call last):
  File "C:\Program Files\Sublime Text 3\sublime_plugin.py", line 210, in on_api_ready
    m.plugin_loaded()
  File "C:\Users\Kostas\AppData\Roaming\Sublime Text 3\Packages\SublimeLinter\sublime_linter.py", line 35, in plugin_loaded
    @legacy_check
  File "C:\Users\Kostas\AppData\Roaming\Sublime Text 3\Packages\SublimeLinter\lint\legacy.py", line 210, in legacy_check
    update_settings(min_version, usr_dir_abs)
  File "C:\Users\Kostas\AppData\Roaming\Sublime Text 3\Packages\SublimeLinter\lint\legacy.py", line 189, in update_settings
    settings = json.load(rf)
  File "./python3.3/json/__init__.py", line 271, in load
  File "./python3.3/json/__init__.py", line 316, in loads
  File "./python3.3/json/decoder.py", line 351, in decode
  File "./python3.3/json/decoder.py", line 367, in raw_decode
ValueError: Expecting property name enclosed in double quotes: line 4 column 5 (char 27)

Eventually, I'd like to try it with SublimeLinter-contrib-eslint, if possible.

pykong · 2017-11-28T22:35:05Z

@darkred Thanks for your interest in the next SublimeLinter.

The current commit is not working, as it serves as a backup push.
Much is WIP as of now, albeit maturing each day.
Try the latest functional commit: 667acf8

Your error log implies a problem with the settings conversion.
Likely you have not created a user settings file for SublimeLinter in your user dir.
Put one containing only {} under \\Packages\\User\\SublimeLinter.sublime-settings'.
See if you can get SublimeLinter to start that way.

@braver
1 - Could you please add to the list of tasks, to check the scenario that no such user settings file is present? (Maybe as a subtask to a main task settings conversion.) Thank you!
2 - What is a good place to track issues/bug reports on the upcoming release?
Should we collect them all in this PR? Or should users open separate issues, we may assign a dedicated label to?

gerardroche · 2017-11-28T23:05:47Z

Regarding more specific scopes, what about prefixing the sublimelinter scope? @FichteFoll

You mean instead of region.yellowish sublimelinter do sublimelinter region.yellowish.

That doesn't work. The reason it doesn't work is because the scopes at the end are more specific and whatever is defined last wins.

It's easier to understand by looking at what problem is being solved.

Let's say plugin sets a region.redish scope for errors. By default ST will use the closest approximation of red in the currently selected color scheme. So far so good. All color schemes are supported using a heuristic which makes things look better by default.

The color scheme wants to provide specific styles for the region.redish. It doesn't matter why. Perhaps the color scheme uses #FF0000 and that means region.redish will use that color, but the color scheme wants to set a lighter shade of red for errors. So it defines a style for the redish region:

    <dict>
        <key>scope</key>
        <string>region.redish</string>
        <key>settings</key>
        <dict>
            <key>foreground</key>
            <string>#F900000</string>
        </dict>
    </dict>

Great. The color scheme has progressively enhanced the redish region colors. But that style is now set for all redish regions. That means all plugins that use the redish regions get that style. That's not bad. That's good. That's what the color scheme wants. However, it wants a slightly different style for sublimelinter. Prefixing won't work i.e. sublimelinter region.redish won't override the previous style. We need a postfix:

    <dict>
        <key>scope</key>
        <string>region.redish sublimelinter</string>
        <key>settings</key>
        <dict>
            <key>foreground</key>
            <string>#FF00000</string>
        </dict>
    </dict>

We might not be done yet. The above style is great. It specifically targets the sublimelinter plugin redish regions. But SL might use redish region for several different elements. Let's imagine a plugin used redish regions for both warnings and errors (as a simple easy to understand example).

    <dict>
        <key>scope</key>
        <string>region.redish warning sublimelinter</string>
        <key>settings</key>
        <dict>
            <key>foreground</key>
            <string>#F500000</string>
        </dict>
    </dict>
    <dict>
        <key>scope</key>
        <string>region.redish error sublimelinter</string>
        <key>settings</key>
        <dict>
            <key>foreground</key>
            <string>#F700000</string>
        </dict>
    </dict>

The color scheme can now control all the elements.

Region.colorish is already in hands of the color scheme, as are things like markup.error. A color scheme can advertise to the user which scopes are suitable for linter highlights, but we’re explicitly putting this in the hands of users.

I'm disappointed. Putting it in the hands of the users is useless. Allowing schemes to progressively enhance the colors of the scheme doesn't take anything away from the user. They still have full control. Allowing color schemes control (without taking it away from the user) enhances users experience without having to ask them to do anything.

darkred · 2017-11-28T23:39:04Z

@pykong Thank you for the help: with the latest functional commit and an empty settings {} it loads ok, no problems.
It's just that sublimelinter-contrib-eslint and sublimelinter-jshint that I tried, both cause an exception in the console, and so they don't work.

So I'll just wait for the next release 👍

pykong · 2017-11-28T23:47:14Z

@darkred You are welcome. What type of exceptions do sublimelinter-contrib-eslint and sublimelinter-jshint cause? State the error logs, so I can gauge whether those errors have anything to do with any changes I made to SL.

If a view file gets reloaded and has hard syntax errors we never actually subscribed, and a subsequent unsubscribe would fail.

braver · 2018-01-04T14:17:09Z

Edit: assuming you're talking about the statusbar busy/linting... indicator.

I think it needs the clarification of what's going on because it's detached from context and you hopefully don't see it a lot. Perhaps we should make all of it configurable so everyone can make it to their taste, or we should have a shared "busy" indicator like they do in Atom. Things can always be improved but we might not always agree on how to do so.

[beta] Fix bug in match dictionary

* Since the RegionStore is only used within this module and as a singleton, we can keep it local. * We had two identical and *static* 'clear' methods. Moved to a normal function 'clear_view'. * The two 'reset' methods were unused. Removed.

Fixes #769

Generally, our data is (0, 0) based. We inc (line:col) by one for the user. Fixes #768

Explicitly test for None'ness and let zeros pass

Cleanup status bar view - Fixup #762

Fixes #767 This is really similiar to how `highlight.full_line()` works.

…o next

Region keys must be unique per view, so a set is the natural type here. By doing that I found that we actually use PROTECTED_REGIONS_KEY twice per draw. So, instead of using a set, I kept the list and check if we actually hold the promise to keep the keys unique.

Replacement for the new queue

Small refactor in highlight

Translate columns when linting using selectors

Since we have a good signal when views actually get unreachable, we just use that `on_pre_close` to trigger a clearance.

Implement simple cleanup for the queue

braver · 2018-01-05T19:22:35Z

Ok, it's clear we won't do anything in SL3 anymore, and if we do we can always branch out again. I'm going to merge this behemoth, it'll make a couple of things easier to manage.

kaste · 2018-01-06T16:23:25Z

😭 I will miss you, branch.

braver · 2018-01-06T16:33:19Z

Yeah, it was a pretty good branch...

👋🏻

braver self-assigned this Nov 13, 2017

This was referenced Nov 25, 2017

Implement color-scheme handling on basis of new .sublime-color-scheme format #639

Closed

Allow Highlight Styling of Individual Warnings/Errors #609

Closed

FichteFoll reviewed Nov 27, 2017

View reviewed changes

braver mentioned this pull request Nov 27, 2017

Major Update: Scheme Generation, Custom Highlights and Cleanups #665

Closed

braver mentioned this pull request Nov 28, 2017

"OSError: resource not found" after install #664

Closed

kaste added 5 commits January 4, 2018 13:11

Fix: Catch KeyError on events.unsubscribe

7caacc0

If a view file gets reloaded and has hard syntax errors we never actually subscribed, and a subsequent unsubscribe would fail.

Early exit on_selection_modified if event didn't came from active_view

9be3eb5

Remove unused code

99bae6a

Do not guard against active_view being None

195d217

Remove keep-alive trick not needed anymore

b4f456b

willrowe and others added 19 commits January 4, 2018 10:08

Make dict comprehension easier to read

b5f90a4

Improved dict comprehension formatting.

f1a3508

Merge pull request #761 from willrowe/match-group-fix

78d4236

[beta] Fix bug in match dictionary

Cleanup highlight

85f18d7

* Since the RegionStore is only used within this module and as a singleton, we can keep it local. * We had two identical and *static* 'clear' methods. Moved to a normal function 'clear_view'. * The two 'reset' methods were unused. Removed.

Remove unused persist.highlights

4e7cfcd

Explicitly test for None'ness and let zeros pass

cb50faf

Fixes #769

Translate start position for the panel view

0368c23

Generally, our data is (0, 0) based. We inc (line:col) by one for the user. Fixes #768

Merge pull request #770 from kaste/fix-769

4e0c722

Explicitly test for None'ness and let zeros pass

Merge pull request #764 from kaste/fixup-762

58bd6c3

Cleanup status bar view - Fixup #762

Translate columns when linting using selectors

3f2c336

Fixes #767 This is really similiar to how `highlight.full_line()` works.

Merge branch 'fix-768' of https://github.com/kaste/SublimeLinter3 int…

e90d936

…o next

Remove RegionStore and KISS

26832db

Merge pull request #763 from kaste/fixup-748

556d128

Replacement for the new queue

Merge pull request #766 from kaste/small-refactor-in-highlight

67e2c10

Small refactor in highlight

Merge pull request #772 from kaste/fix-767

c8b7390

Translate columns when linting using selectors

Implement simple cleanup for the queue

069bb75

Since we have a good signal when views actually get unreachable, we just use that `on_pre_close` to trigger a clearance.

Merge pull request #773 from kaste/proper-queue-cleanup

13a27f5

Implement simple cleanup for the queue

implements settings validation via jsonschema

8a830da

braver merged commit 8a830da into master Jan 5, 2018

braver deleted the next branch January 5, 2018 19:43

Uh oh!

WIP: Next #666

WIP: Next #666

Uh oh!

Conversation

braver commented Nov 13, 2017 • edited by pykong Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jrappen commented Nov 22, 2017

Uh oh!

braver commented Nov 22, 2017

Uh oh!

jrappen commented Nov 22, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

braver commented Nov 22, 2017

Uh oh!

FichteFoll commented Nov 23, 2017

Uh oh!

braver commented Nov 23, 2017

Uh oh!

FichteFoll commented Nov 23, 2017

Uh oh!

FichteFoll commented Nov 23, 2017

Uh oh!

braver commented Nov 23, 2017

Uh oh!

gerardroche commented Nov 23, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

FichteFoll commented Nov 23, 2017

Uh oh!

braver commented Nov 23, 2017

Uh oh!

FichteFoll Nov 27, 2017

Choose a reason for hiding this comment

Uh oh!

braver Nov 27, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

FichteFoll Nov 27, 2017

Choose a reason for hiding this comment

Uh oh!

pykong commented Nov 27, 2017

Uh oh!

darkred commented Nov 28, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pykong commented Nov 28, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gerardroche commented Nov 28, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

darkred commented Nov 28, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pykong commented Nov 28, 2017

Uh oh!

braver commented Jan 4, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

braver commented Jan 5, 2018

Uh oh!

kaste commented Jan 6, 2018

Uh oh!

braver commented Jan 6, 2018

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

10 participants

braver commented Nov 13, 2017 •

edited by pykong

Loading

jrappen commented Nov 22, 2017 •

edited

Loading

gerardroche commented Nov 23, 2017 •

edited

Loading

braver Nov 27, 2017 •

edited

Loading

darkred commented Nov 28, 2017 •

edited

Loading

pykong commented Nov 28, 2017 •

edited

Loading

gerardroche commented Nov 28, 2017 •

edited

Loading

darkred commented Nov 28, 2017 •

edited

Loading

braver commented Jan 4, 2018 •

edited

Loading