WIP: Remove the syntaxes key from the client configs

[rwols]

close #823
related #540

Make the arg optional in LanguageConfig,
so as to not disturb LanguageHandlers in the wild.

Cache the lookup, because we're loading an entire file and doing
a regex search in it.

This is a WIP, and I'd like to ask @tomv564 for directions on how to handle the importing of sublime in configurations.py now (or wherever), making the pytest tests fail. Perhaps we can move more tests to UnitTesting instead?

• Cache the regex search. It's costly!
• Account for old .tmLanguage files.

[jfcherng]

I encountered noticeable performance problem with this branch today.

Installed Packages

• BracketHighlighter
• LSP (remove-syntaxes-key branch)
• RestructuredText Improved

The minimal reproducer is only these 3 packages installed.

Reproducer

https://github.com/jfcherng/reproducer-lsp-remove-syntaxes-key-performance

To reproduce, use the Data/ as the portable configuration and then open test.rst.

Video Recording

• remove-syntaxes-key branch: https://streamable.com/8ombj
• 0.9.3: https://streamable.com/ybk79

[rwols]

Thanks for trying it out!

The problem is, I think, that we're loading a tmLanguage file, not a sublime-syntax file. So:

        match = re.search(r'^scope: (\S+)', sublime.load_resource(syntax), re.MULTILINE)
        if not match:
            return False

On every keypress this will keep loading the entire syntax file and doing a regex search in it :)
We should have extra logic to account for .tmLanguage files.

[jfcherng]

It looks like you are right. If I convert the syntax into a .sublime-syntax one, the problem no longer persists.

[tomv564]

The implementation assumes only 1 scope can be found, I think you likely want to store the "0 scopes" result and also handle the multiple scopes (embedded languages) case.

Which brings us to @rchl 's case, where some language servers don't handle being sent non-javascript documents that happen to have source.js embedded, or like he has two servers for source.js - one for Vue and one for regular Javascript, and doesn't want to run both on every file containing source.js a scope.

[rwols]

or like he has two servers for source.js - one for Vue and one for regular Javascript, and doesn't want to run both on every file containing source.js a scope.

So he sets up his configs like this:

{
    "command": ["vue-ls"],
    "languageId": "vue",
    "scopes": ["text.html.vue"]
},
{
    "command": ["javascript-typescript-langserver", "--stdio"],
    "languages": [
        {"languageId": "js", "scopes": ["source.js"]},
        {"languageId": "jsx", "scopes": ["source.jsx"]}
    ]
}

The base scope of a Vue file is text.html.vue. Only vue-ls runs for these kinds of files.
The base scope of a JS file is source.js or source.jsx. Only javascript-typescript-langserver runs for these kinds of files.

Additionally you can add eslint as a second server for both files (I'm not sure if eslint supports Vue files, but let's pretend that it does for the example):

{
    "command": ["eslint"]
    "languages": [
        {"languageId": "js", "scopes": ["source.js"]},
        {"languageId": "vue", "scopes": ["text.html.vue"]}
    ]
}

Now he'll run vue-ls+eslint for Vue files, and javascript-typescript-langserver+eslint for JS files.

Moreover, it won't matter whether someone is using Babel Sublime Syntax, or Ecmascript Syntax, as all of these syntaxes have source.js as their base scope cleverly chosen so that color schemes don't break.

Future roadmap if we agree on this scopes approach: Replace the scopes list by a selector string. Because this:

"scopes": ["source.foo", "source.bar"]

will give the same selector score as

"selector": "source.foo | source.bar"

The current state of this PR has some bugs with respect to project switching, please don't suddenly merge.

[rwols]

Also there seems to be some confusion on what scopes originally did. It seems it was intended to select the most suitable session in case there were multiple, when a "feature" only wanted a single session. But the default configurations don't encourage this usage of the scopes key, as all of them just have the base scope assigned to them. There is also not a very good explanation for it in the documentation.

[rwols]

Someone on Discord pointed out that there's sublime.list_syntaxes() for build 4050 and onward. It works like this:

>>> from pprint import pprint
>>> pprint(sublime.list_syntaxes())
[{'hidden': False,
  'name': 'R',
  'path': 'Packages/R/R.sublime-syntax',
  'scope': 'source.r'},
 {'hidden': False,
  'name': 'friendly interactive shell (fish)',
  'path': 'Packages/sublime-fish-shell-improved/fish.tmLanguage',
  'scope': 'source.shell.fish'},
 {'hidden': False,
  'name': '',
  'path': 'Packages/CMakeBuilder/Syntax/Configure.sublime-syntax',
  'scope': 'text.log'},
 {'hidden': False,
  'name': 'Perl',
  'path': 'Packages/Perl/Perl.sublime-syntax',
  'scope': 'source.perl'},
 {'hidden': False,
  'name': '',
  'path': 'Packages/CMakeBuilder/Syntax/Make.sublime-syntax',
  'scope': 'output.build.make'},
   ...

We can use that information to extract the base scope like this:

>>> syntax='Packages/PowerShell/Support/PowershellSyntax.tmLanguage'
>>> next(filter(lambda d: d['path'] == syntax, sublime.list_syntaxes()))['scope']
'source.powershell'

So it looks like this is yet another problem that ST4 will solve more elegantly.

[predragnikolic]

Why is this closed 🙂

[rwols]

I want to make a new one using the ST4 api

[james2doyle]

@rwols is there some info out about ST4? I googled around and didn’t find anything concrete

[jfcherng]

@james2doyle it's only published in the official discord chatroom for early test purpose at this moment.