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

ST should re-check TextChangeListener.is_applicable when a buffer's view's settings change #6117

Open
Calvin-L opened this issue Aug 26, 2023 · 2 comments

Comments

@Calvin-L
Copy link

Calvin-L commented Aug 26, 2023

Description of the bug

A common way to implement TextChangeListener.is_applicable is to consult the buffer's primary view's settings:

class MyViewEventListener(sublime_plugin.ViewEventListener):
    @classmethod
    def is_applicable(cls, settings):
        return # ... check some property of the settings ...

class MyTextChangeListener(sublime_plugin.TextChangeListener):
    @classmethod
    def is_applicable(cls, buffer):
        return MyViewEventListener.is_applicable(buffer.primary_view().settings())

For instance, this pattern was mentioned and upvoted here.

However, ST does not re-check TextChangeListener.is_applicable when the settings change, even though it does re-check ViewEventListener.is_applicable when the settings change.

This is a problem for plugins that only want text change events on certain kinds of syntax. Consider e.g. Calvin-L/sublime-coq-plugin#9, in which the following sequence of actions thwarts the plugin's attempts to listen to text changes in Coq files:

  1. make a new file
  2. set the syntax to Coq
  3. interact with the plugin --- does not work properly since ST only checks TextChangeListener.is_applicable after step 1, when the file is not yet a Coq file

Steps to reproduce

Create text_change_plugin_example.py in <ST>/Packages/User/:

import sublime
import sublime_plugin


class MyViewEventListener(sublime_plugin.ViewEventListener):

    @classmethod
    def is_applicable(cls, settings):
        # only listen to events on Python3 files
        print(f"Checking applicability with syntax={settings.get('syntax')}")
        return settings.get("syntax", "").endswith("/Python.sublime-syntax")


class MyTextChangeListener(sublime_plugin.TextChangeListener):

    @classmethod
    def is_applicable(cls, buffer):
        return MyViewEventListener.is_applicable(buffer.primary_view().settings())

    def on_text_changed(self, changes):
        print(f"Handling {len(changes)} changes")

Then

  1. Create a new file
  2. Set the syntax to Python
  3. Make some modifications

Expected behavior

There should be "Handling X changes" messages in the console.

Actual behavior

There are no "Handling X changes" messages, since MyTextChangeListener was not given a second chance to advertise its applicability after step 2.

Sublime Text build number

4152

Operating system & version

macOS 12.6.8

(Linux) Desktop environment and/or window manager

No response

Additional information

No response

OpenGL context information

No response

@BenjaminSchaaf
Copy link
Member

This is intentional. TextChangeListener.is_applicable can change its output due to any number of things; you're expected to use TextChangeListener.attach and detach to manage the listener yourself if more complex behavior is needed.

@Calvin-L
Copy link
Author

Ah, gotcha. If that's the official word then I'm prepared to close this and adopt attach/detach in my plugins.

But, if you have time to clarify a little:

In my current understanding, I don't see any correct implementation of TextChangeListener.is_applicable except return True or return False. Every interesting property of a buffer except its id() is mutable---if ST never re-checks is_applicable, then error-prone attach/detach code looks like the only correct way to restrict which buffers are being listened to.

Obviously ST can't re-check is_applicable for every change to every possible property of a buffer or its views, but re-checking on settings changes seems (to me) like a reasonable middle-ground. It would have nice symmetry with how ST already re-checks ViewEventListener.is_applicable on settings changes.

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

No branches or pull requests

2 participants