Add Semantic Higlighter

[kapitanluffy]

Plugin for semantic highlighting to variables

[packagecontrol-bot]

Automated testing result: WARNING

Repo link: Semantic Highlighter
Results help

Packages added:
  - Semantic Highlighter

Processing package "Semantic Highlighter"
  - WARNING: The binding ['ctrl+shift+l'] is also defined in default bindings but is masked with a 'context'
    - File: Default (Windows).sublime-keymap
  - WARNING: The binding ['ctrl+shift+e'] is also defined in default bindings but is masked with a 'context'
    - File: Default (Windows).sublime-keymap

[packagecontrol-bot]

Automated testing result: SUCCESS

Repo link: Semantic Highlighter

Packages added:
  - Semantic Highlighter

Processing package "Semantic Highlighter"
  - All checks passed

[FichteFoll]

Why do you only define key bindings for Windows?

Setting change event handlers are registered for the entire settings objected and keyed by a string id for later removal, not for some setting name you provide.

As such, you must also unregister setting change handlers in a plugin_unloaded function as they will otherwise persist even when the package is unloaded.

What is the point of this context handler?

[FichteFoll]

@kapitanluffy ping

[kapitanluffy]

Setting change event handlers are registered for the entire settings objected and keyed by a string id for later removal, not for some setting name you provide.

Hey sorry I got swamped with work. Can you elaborate on this one?

[FichteFoll]

It's like adding a key to a dictionary of callbacks that you can remove later by specifying the same key. Adding the same callback twice with different keys will result in the callback being called twice for every change.

[kapitanluffy]

Hi @FichteFoll I updated the package. Can we reopen this PR?

[packagecontrol-bot]

Automated testing result: SUCCESS

Repo link: Semantic Highlighter

Packages added:
  - Semantic Highlighter

Processing package "Semantic Highlighter"
  - All checks passed

[FichteFoll]

Setting change event handlers are registered for the entire settings objected and keyed by a string id for later removal, not for some setting name you provide.

This is still handled incorrectly, although it doesn't break with how it is currently. I suggest to just read the corresponding settings from the settings object directly when you want to use them instead of using a settings listener.

What is the point of this context handler?

[kapitanluffy]

Setting change event handlers are registered for the entire settings objected and keyed by a string id for later removal, not for some setting name you provide.

This is still handled incorrectly, although it doesn't break with how it is currently. I suggest to just read the corresponding settings from the settings object directly when you want to use them instead of using a settings listener.

What is the point of this context handler?

I rewrote the whole thing. The context handler is for the keybindings. Let me know if there's a better way of doing that :)

Also, for the settings listener issue, do you mean that on add_on_change it fires the callback everytime any setting is changed on that Setting object?

So with that in mind, I should do:

preferences = sublime.load_settings('Preferences.sublime-settings')
preferences.add_on_change('my_plugin_key', my_callback_for_changes)

[kapitanluffy]

Tried reading the settings directly instead of registering a callback but I realized that it would read the settings file with every cursor move (which I think is very inefficient).

I think I understood what you are saying now regarding the listeners though.

[FichteFoll]

Also, for the settings listener issue, do you mean that on add_on_change it fires the callback everytime any setting is changed on that Setting object?

Yes.

I rewrote the whole thing. The context handler is for the keybindings. Let me know if there's a better way of doing that :)

The current implementation is still the same and a no-op. It will always match when the corresponing "key" is provided, in which case you might as well omit it entirely.

Tried reading the settings directly instead of registering a callback but I realized that it would read the settings file with every cursor move (which I think is very inefficient).

Hm, that may indeed be a valid reason to have a cache in form of a native Python dict. You could time/benchmark it to be sure, but I understand the reasoning.

[kapitanluffy]

Also, for the settings listener issue, do you mean that on add_on_change it fires the callback everytime any setting is changed on that Setting object?

Yes.

I rewrote the whole thing. The context handler is for the keybindings. Let me know if there's a better way of doing that :)

The current implementation is still the same and a no-op. It will always match when the corresponing "key" is provided, in which case you might as well omit it entirely.

Tried reading the settings directly instead of registering a callback but I realized that it would read the settings file with every cursor move (which I think is very inefficient).

Hm, that may indeed be a valid reason to have a cache in form of a native Python dict. You could time/benchmark it to be sure, but I understand the reasoning.

I removed the keybinding contexts for now. I fully understand what contexts are for now after reading about it on the docs.

I also removed a mousemap file that I forgot included in the repo.