aziz/SublimeANSI

event listener callback error

heyblinken opened this issue · 26 comments

I keep getting an error in detect_syntax_change because view == None, but you are referencing it anyway. Please add check to see if view == None and return if True.

    if view == None:
        return

Also, probably should add this check in assign_event_listener as well.

I didn't see that but it may happen.

view.settings().add_on_change("CHECK_FOR_ANSI_SYNTAX", lambda: self.detect_syntax_change(view))

I am not sure how a callback with a local variable as a parameter will be called in a listener (undefined behavior?).

Since view is available as a member in a listener, we may use the following codes?

    def detect_syntax_change(self):
        v = self.view
        if v.settings().get("syntax") == "Packages/ANSIescape/ANSI.tmLanguage":
            v.run_command("ansi")
        elif v.settings().get("ansi_enabled"):
            v.window().run_command("undo_ansi")

Here is the actual error output:

Traceback (most recent call last):
File "ansi in C:\Program Files\Sublime Text 3\Data\Installed Packages\ANSIescape.sublime-package", line 160, in
File "ansi in C:\Program Files\Sublime Text 3\Data\Installed Packages\ANSIescape.sublime-package", line 168, in detect_syntax_change
AttributeError: 'NoneType' object has no attribute 'run_command'

Actually, looking at the error again, i think the view is valid, but view.window() == None.

So we would something like this:
if( view == None or view.window() == None ):
return

Interesting... from line 168, so view is valid but view.window() returns None. Kind of wtf to me 😀

window() Window Returns a reference to the window containing the view.

Since error logs are repeating, I guess we only need a few lines and leave this post more clear.

Yeah, i think the console's view is one of the views where view can be valid, but it doesn't reference a valid window(). I don't know if that's the view its triggering the callback on, but I've ran into this issue before.

Wrong again... I did a test using the following code:

        print( getattr( view, "run_command", None))
        print( getattr( view, "window", None) )
        print( getattr( view.window(), "run_command", None) )

and got these results:

       <bound method View.run_command of <sublime.View object at 0x000000000437A4E0>>
       <bound method View.window of <sublime.View object at 0x000000000437A4E0>>
       None

So it looks like view and view.window() are valid and the problem is that view.window() doesn't have a run_command() function.

I've tried several more attempts at catching this issue and it appears that the view object is either getting corrupt or its not valid in the first place since its not provided as self.view and passed in instead. Likewise, in my scenario, every time i click on a different view, i'm hitting the callback ~ 6 times. So it seems like this isn't a very efficient method of detecting if the syntax changed.

Likewise, where the error is occurring ( inside of elif view.settings().get("ansi_enabled"): ), i don't even have any active views that are using the ansi extension, so it shouldn't be trying to call view.window().run_command("undo_ansi") in the first place

@heyblinken Thank you for all the info and debug, it it greatly appreciated. Unfortunately I have been unable to reproduce this issue; would it be possible to provide some steps on how to reproduce this issue and also some more info on the setup/environment that you are using?

It is probably worth pulling the latest version of SublimeANSI from github, as there have been several commits that are relevant to the area of code that you are looking at:

commit a2307ae Reduce unnecessary spamming of ansi text command.
commit 41c4b67 Remove old event listener before assigning a new one.
commit 98a6727 Added print verbose debug method.
commit 12d2b90 Even nicer verbose output.

The first two commits should resolve the issue of the ansi callback and ansi command being invoked multiple times; and the latter two will hopefully better debugging information.

@jfcherng I have tried you suggestion:

    def assign_event_listener(self, view):
        view.settings().add_on_change("CHECK_FOR_ANSI_SYNTAX", lambda: self.detect_syntax_change())

    def detect_syntax_change(self):
        view = self.view

However the following error occurs:

AttributeError: 'AnsiEventListener' object has no attribute 'view'

Is view not a member of listener or are we just doing something wrong?

@matt1003 Sorry for my bad memory. An event listener class has no self.view indeed.

I've pulled down your latest changes and will test them. My setup is complicated and i wasn't able to produce this bug very often. It would occasionally occur when i would run one of my scripts. I don't believe its my script as I started noticing it once i started using your plugin. Likewise, the script that i run seems to fail as well, right after your script has the issue... I'll let you know if i encounter this issue again with ac1f413.

got it to happen again... i wondering if you should be passing view into detect_syntax_change as jfcherng mentioned earlier. i don't know if settings()view_on_change() was meant to tell you what view modified a setting, or it would provide it.

Traceback (most recent call last):
  File "ansi in C:\Program Files\Sublime Text 3\Data\Installed Packages\ANSIescape.sublime-package", line 241, in <lambda>
  File "ansi in C:\Program Files\Sublime Text 3\Data\Installed Packages\ANSIescape.sublime-package", line 256, in detect_syntax_change
AttributeError: 'NoneType' object has no attribute 'run_command'
Traceback (most recent call last):
  File "ansi in C:\Program Files\Sublime Text 3\Data\Installed Packages\ANSIescape.sublime-package", line 241, in <lambda>
  File "ansi in C:\Program Files\Sublime Text 3\Data\Installed Packages\ANSIescape.sublime-package", line 256, in detect_syntax_change
AttributeError: 'NoneType' object has no attribute 'run_command'
s =  False
Traceback (most recent call last):
  File "C:\Program Files\Sublime Text 3\Data\Packages\******.py", line 98, in get_rgn_info
    if not "settings" in s:
TypeError: argument of type 'bool' is not iterable

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "ansi in C:\Program Files\Sublime Text 3\Data\Installed Packages\ANSIescape.sublime-package", line 241, in <lambda>
  File "ansi in C:\Program Files\Sublime Text 3\Data\Installed Packages\ANSIescape.sublime-package", line 256, in detect_syntax_change
AttributeError: 'NoneType' object has no attribute 'run_command'

Ok, i added some traceback debugging to detect_syntax_check and discovered 3 things:

  1. i have Bracket Highlighter plugin installed, and so that triggers detect_syntax_check like crazy when you focus a different view.
  2. The script i'm running that causes the issue is calling sublime.active_window().project_data(), which is triggering detect_syntax_change. not sure why
  3. When a view is closed, you are not removing the callback for it, which is where i think the error is actually happening. if you look in the traceback, the window.id() value of 76 is bogus. Likewise, i only have 2 open views and ansi isn't one of them
detect_syntax_change:
 (15, 2)
    File "C:\Program Files\Sublime Text 3\Data\Packages\test\test.py", line 793, in <lambda>
    self.view.window().show_input_panel("Enter load Options:", prompt, lambda t: self.set_user_input(t), None, None)
    File "C:\Program Files\Sublime Text 3\Data\Packages\test\test.py", line 842, in set_user_input
    self.load_str = self.get_load_string( ld_trgt, args )
    File "C:\Program Files\Sublime Text 3\Data\Packages\test\test.py", line 927, in get_load_string
    ( data, line ) =  test.test.get_rgn_info( a, b)
    File "C:\Program Files\Sublime Text 3\Data\Packages\test\tests.py", line 97, in get_rgn_info
    s = sublime.active_window().project_data()
    File "C:\Program Files\Sublime Text 3\sublime.py", line 183, in active_window
    return Window(sublime_api.active_window())
    File "ansi in C:\Program Files\Sublime Text 3\Data\Installed Packages\ANSIescape.sublime-package", line 245, in <lambda>
    File "ansi in C:\Program Files\Sublime Text 3\Data\Installed Packages\ANSIescape.sublime-package", line 259, in detect_syntax_change


File: "F:\git\My_Projects\test\ansi in C:\Program Files\Sublime Text 3\Data\Installed Packages\ANSIescape.sublime-package", line 272, window: 76, view: 15, file: not named, time: 2016-04-15 15:01:35
    Syntax change detected (running undo command).
    detect_syntax_change done

So i think you need to add support for removing callbacks when a view is closed and try to remove passing view to detect_syntax_change and maybe use sublime.active_window().active_view() or maybe search through sublime.active_window().views.

@heyblinken - really nice work! Just added callback removing on_close.

I'm sceptical about using active_window() - settings change listener is assign for each view, and it's callback shouldn't change other views.

I pulled down 1e55d9d and still had the same errors. Looking at on_close changes, i think you need to change it to on_pre_close() so that 'view' is still valid. I tried adding your debug() function to on_close and it got errors trying to resolve view.id().

done

I changed it to on_pre_close() and still am having the same problem. Likewise, i also am noticing similar errors when i open up multiple projects... I'm starting to wonder if its my setup, if no else is seeing these issues.

I added check for none window - hopefully this will work.

I checked out 8c47400 and still had it happen in detect_syntax_change.

This doesn't work because in my situation, because view.window != None. Likewise, it also doesn't contain values that correspond to any actual window or view. i.e. i've seen view.window().id == 85

if view.window is None:
           self._clear_view_listeners(view)
           print("\tansi detected error")
           return

The only way i've been able to detect this issue is to explicitly make sure the view id and window id exist.

    def detect_syntax_change(self, view):
        found = False
        if( not view.window() == None ):
            for w in sublime.windows():
                if( found ):
                    break

                if w.id() == view.window().id():
                    for v in w.views():
                        if( v.id() == view.id() ):
                            found = True
                            break

        if( found == False ):
            print("detect_syntax_change: invalid view")
           # commented out return so that i could still see the failure
           # return

This is big wtf for me. Can you send error logs?

detect_syntax_change: invalid view
Traceback (most recent call last):
  File "ansi in C:\Program Files\Sublime Text 3\Data\Installed Packages\ANSIescape.sublime-package", line 241, in <lambda>
  File "ansi in C:\Program Files\Sublime Text 3\Data\Installed Packages\ANSIescape.sublime-package", line 314, in detect_syntax_change
AttributeError: 'NoneType' object has no attribute 'run_command'

Likewise, i had more primitive checks that never would catch the issue. If you look in the above log, only the first round of checks that use print("detect_syntax_change: invalid view"), actually catch the issue.

    def detect_syntax_change(self, view):

        found = False
        if( not view.window() == None ):
            for w in sublime.windows():
                if( found ):
                    break

                if w.id() == view.window().id():
                    for v in w.views():
                        if( v.id() == view.id() ):
                            found = True
                            break

        if( found == False ):
            print("detect_syntax_change: invalid view")
            # return

        if( view == None or view.id() == None or view.window() == None or view.window().id() == None or view.window == None or view.window().id() > 5 ):
            print("detect_syntax_change error:" )
            print("\tview", view )

            if( not view == None ):
                print("\tname", view.file_name() )
                print("\tview.id()", view.id() )
                print("\tview.window()", view.window() )
                print("\tview.window", view.window )

            if( not view.window() == None ):
                print("\tview.window.id()", view.window().id() )

            print("traceback\n")
            for line in traceback.format_stack():
                print(line.strip())
            print("\ttrackback done\n\n")

        if view.window is None:
            self._clear_view_listeners(view)
            print("\tansi detected error")
            return
        if view.settings().get("ansi_in_progres", False):
            return
        if view.settings().get("syntax") == "Packages/ANSIescape/ANSI.tmLanguage":
            if not view.settings().has("ansi_enabled"):
                debug(view, "Syntax change detected (running ansi command).")
                view.run_command("ansi", args={"clear_before": True})
        else:
            if view.settings().has("ansi_enabled"):
                debug(view, "Syntax change detected (running undo command).")
                view.window().run_command("undo_ansi")

Did anyone ever confirm if it is ok to save of "view" when assigning the event listener's? I know you added support to un register the callbacks, but is it possible that sublime is changing/corrupting your passed in view, which is why it has bogus data in it?

I still don't get it. I suppose that line 314 is this one (the last line):

                view.window().run_command("undo_ansi")

I just put another patch - hopefully it will work this time. It does the same as your first check - check if view.window() is in sublime.windows() and if view is in view.window().views()

thanks, i'll give it a shot

I just dig in and found that there is better way of detecting syntax change with on_text_command and we don't have to pass view to check it. However we are still using view.settings().add_on_change to detect file reload.

I've been testing 143a6f8 and haven't seen any issues so far... looks good, thx.

It appears that the view validation that was introduced in commits b5ceb21 and 143a6f8 may be a little too aggressive. It turns out that view.window will be set to None while a tab is being shifted from one position to another. This somewhat makes sense, as the view doesn't really belong to any specific window while it is being moved around.

The problem is that _is_view_valid is invoked while the view is being moved; hence it sees view.window == None and assumes the view is invalid (thus removing the ansi event listeners). As a result, views that have been moved will no longer respond to syntax changes via the sublime menu.

This issue is still valid, please see #43 (comment).