showing changes

[mg979]

Don't accept this PR. It's here to make a diff so I can show the the changes

[oleg-shilo]

Seems OK. I also tested it quickly no new issues. Will wait for you for those 1 and 3(4) form the testing feedback.

BTW I only mentioned 3(and 4) to you because I thought that it was some how introduced with the latest changes. But I can be wrong. Please let me know if the focus management problems were introduced by some of my old code and I will have a look at it independently.

[mg979]

I am not sure about 3 and 4. Maybe you can try to checkout master and see if it happens. I'm trying to add support for your favorites plugin, so that they can stay in the same group. If it works in an uncomplicated way, I'll show it.

[oleg-shilo]

Not a problem. Just find the compromise for 1 and I will handle the rest. I will not have the opportunity until tomorrow though. Thank you.

[mg979]

Just one thing if things are ok and there are not surfacing bugs.. If you plan to merge it on master sooner or later(maybe after some more testing) maybe you can avoid to merge all commits because they're a lot, I don't know how these things work but I think it would be better if there were a single commit with all the new files, the single commits would stay in the dev branch.

[oleg-shilo]

Agree. If I cannot figure out how to collapse the commits I will just brutally overwrite master source code with the dev's one and commit with an informative comment. I am sure it's not very nice things to do but... no big deal for me.

[oleg-shilo]

Cool, it was fun. I've done it. My Git has definitely improved. :)

I managed to squash all your PR related commits (along with mine) and now dev has the all latest changes. Note I also brought manually the 'code_map.py' from your just_to_see branch including my fix for that Focus Management problem (yes it is fixed now).

Thus I suggest you sync and pull all the changes from dev if you still plan to do any work for it. I am happy with the codebase and almost ready to push to master and make the release.

I'll wait a few days just in case if you want to push something last minute.

The only one remaining cosmetic issue is that "view is not bound" error message:

The message is totally OK but the suggestion part of it is not. Trying to refresh the map will fail to generate the map in 100% cases (the view document is just incompatible). Thus suggesting it to the user is not entirely appropriate.

I can remove the "suggestion" part of the message (in dev) however I wanted you to confirm that my above interpretation is correct.

[mg979]

The only one remaining cosmetic issue is that "view is not bound" error message

The message is there, you can change it with a simple "Not a file.". Yes right now only files whose syntax is among the defined syntaxes in the settings can be mapped, not even universal mapper. I think it's better because one could map big/wrong files by mistake.

I'll wait a few days just in case if you want to push something last minute

The only thing I can think of right now is this timeout, I wonder if it were better placed directly in the command because wrong stuff in listeners crashes often Sublime. But it didn't give me problems for now, so maybe just to keep in mind if problems arise.

[mg979]

There is some problem with the latest changes you made. It makes ST very unstable when switching projects in my installation(plugin host stops working and even crash). Surely it has to do with the listener. Could you please just put in dev only the changes up to this fake PR without your latest changes? If necessary, delete dev branch, create it again from master and squash commits again, but without your changes, there is something that must be fixed.

[oleg-shilo]

Done. The dev now contains your original just_to_show copy of _codemap.py.

Could you please just put in dev only the changes up to this fake PR without your latest changes?

Note that I only manually brought _codemap.py (with my changes) from just_to_show to dev. Assuming that your changes are already in dev. Please let me know if it is the case. May be it is the problem?

Though I want to actually understand and fix the cause of the problem. Surely crashing is worse than uncontrollable active document switching but I'd rather have them both properly fixed. Can you please provide a test-case that demonstrates the problem (if possible)? So I can investigate.

[mg979]

Changing functions in a listener and not restarting ST may be alone a cause for crashes, so maybe this was the only cause, but maybe not. In any case, it's better to have in dev what's sure not to cause issues, sorry I was very alarmed by the crash. If there's some refinement to be made, we start from something that was running smoothly. I hope you agree :)

[mg979]

I don't know exactly what happened, I will run some tests. I think the main problem is that when you switch project, the layout/groups/files change very fast, and if the listener tries to close some group or do something on stuff that doesn't exist anymore, it can break something.

[oleg-shilo]

Yes I agree but I am not sure that the current dev is running smoothly. Even though I removed my "Focus Management" changes.

I checked again that "Focus Management" change and I can confirm that it practically did not involve listener. Only a non-aggressive prevention of the map refresh when the source and the map view are in the same group. Thus quite possible the the cause was already in the solution.

Anyway, I will wait for your report on the dev stability and resume gentle reintroduction of "Focus Management" change when the branch is indeed running smoothly.

As for calling set_timeout_async from listener I totally agree that it needs to go to the command.

[oleg-shilo]

Forgot to mention. I manually verified (with KDif3) that the content of dev and just_to_show is currently identical.

[mg979]

It was probably a false alarm, sorry. I should have restarted ST. Anyway I think it's better not to add new stuff together with that large commit. I made a new commit here, with your changes plus some little fixes (included timeout in command).

[oleg-shilo]

Anyway I think it's better not to add new stuff together with that large commit.

Amen to that :)

I made a new commit here, with your changes plus some little fixes (included timeout in command).

Can you please confirm that the branch just_to_see is ready and I can try to smoke test it?

[mg979]

Amen to that :)

Thanks for understanding :)

I made another small change, now it's ready if you can test it

[oleg-shilo]

Something went wrong. The map is never generated. Such a dramatic failure is most likely to my environment conditions. Investigating now...

[mg979]

I forgot to add that I disabled map generation when sidebar is open, because it creates maps as you scroll files. Maybe I should remove that. If you close the sidebar map generation works.

[oleg-shilo]

OK, found the reason. The new condition

[oleg-shilo]

Yes it is the one :) I tracked it down buy myself

[mg979]

Maybe it's not been a good idea. Should it be removed?

[oleg-shilo]

I think so.

[oleg-shilo]

BTW, what is this Window.transient_view_in_group? I cannot find any documentation on it.

[mg979]

Sorry, really a bad idea, it wouldn't work at all with open sidebar.

[mg979]

Window.transient_view_in_group is the transient view that is created by quick panel. For example "go to anything" command (ctrl+p), or File History plugin. They open a file and CodeMap maps it without that condition. I was looking for a way to prevent that behaviour, I found this command in another plugin.

[mg979]

By the way, are your changes working with this version?

[oleg-shilo]

Seems like a good improvement.

Yes my changes work but with one exception. It still changes the active doc if the toggling (e.g. closing map view) was triggered while another doc from the map group has focus.

I feel like it is something that I have fixed ("Focus Management"). Thus if you feel that it is unlikely that it's introduced with your latest commits, I can redo it. But of course, only when you signal it's OK for me to step in :)

[mg979]

I changed a bit on_activated_async(), maybe this is the cause. I don't think I want to make more changes, from my part, for now, I can't think of anything else.

[oleg-shilo]

OK then. It is already reasonable stable and addresses ~70% of that FocusManagement issues. The rest is a very infrequent use-cases. Let's seal it for now. Will keep using/testing for a couple of days and if no other issued detected then we will release it.

In background I will investigate that 30% of problems but will introduce the fix (if found) only after that graceful period.

Do you want me to squash your commits?

[oleg-shilo]

Oh, sorry I cannot. Only you can do. It's not my branch.

[mg979]

Maybe I understood what happens. I cannot test it because I don't know how to reproduce it but:

when you close through command (alt+m, alt+m) this is run and updates current view for focus_source_code(): CodeMapListener.active_view = view

If you close the map with the mouse this doesn't run, so CodeMapListener.active_view is still the old view.

[mg979]

My commits are done for now. How is it that I can squash them? can't you just take the file and overwrite, then squash them?

[oleg-shilo]

Txs. Have a nice day.

[mg979]

Ok nice day to you too.

[mg979]

By the way I'm liking your Favorites plugin, I thought you could make this:

class show_favorites(sublime_plugin.TextCommand):
    def run(self, edit, focus=False):

        w = sublime.active_window()
        fav_view = get_panel_view()
        if focus and fav_view:
            w.focus_view(fav_view)
            w.focus_view(self.view)
        else:
            show_panel.run(self, edit)

Then a keybinding:

{ "keys": [...], "command": "show_favorites", "args": {"focus": true} },

So that you can put map and favorites in the same group, then synch map to focus on it, and this command to focus on favorites.

Also this in class show_panel:

        if not panel_view:

            panel_group = -1
            last_group = window.num_groups()-1
            # look for CodeMap in last group
            for view in window.views_in_group(last_group):
                if os.path.basename(view.file_name()) == 'Code - Map':
                    panel_group = last_group

            # CodeMap not found
            if panel_group == -1:
                show_in_new_group = settings().get("show_in_new_group", True)

                if not show_in_new_group:
                    if groups == 1:
                        set_layout_columns(2)
                        groups = window.num_groups()

                else:
                    panel_group = create_panel_group()

If then you have keybindings like:

{ "keys": ["alt+f", "alt+f"], "command": "show_favorites", "args": {"focus": true} },

This opens Favorites in the same CodeMap group, but if you press it again it just focuses on it. Old command to close if you need (or use command palette):

{ "keys": ["alt+f", "alt+x"], "command": "show_favorites"

[oleg-shilo]

That last 30%. Solved. Was a simple case of missing alone_in_group test:

def reset_layout(reduce):
    ...
    map_view = get_code_map_view()
    alone_in_group = len(w.views_in_group(get_group(map_view))) == 1

    if map_view and alone_in_group:
        w.set_view_index(map_view, 0, 0)
    ...

Though I will wait with it until you create the PR so I can merge your just_to_see into my dev.

[mg979]

There's still a problem before I can make a PR. It's not very stable yet when I switch projects. I'll try to find a fix because it's a very big problem, I don't want crashes. I also have them with the commit that I considered stable, so I have to find the cause.

[oleg-shilo]

Txs, I will review Favorites when we are done with this one. BTW, with your future PR, there is no pressure at all. So take your time.

I also discovered another nasty artifact. If node, just below the currently selected one, is double-clicked it does not trigger the new selection reliably: Though if it is not the next one but the further below node then it's all good.

When I reflect at all this experience I cannot stop being disappointing with ST3 almost religious stubbornness with resisting true panels. :( I like so much more VSCode for that.

[mg979]

Though if it is not the next one but the further below node then it's all good.

Mmm it's actually happening with all of them, it only depends on how fast you click another line. But not really always. It's probably because there are some async timeouts that need their time, maybe there are timeouts in sequence that also control focus. I don't think it's a big issue. I remember I set a timeout because it wasn't selecting the line anymore, just a word. I see if I can improve this too.

[oleg-shilo]

I just fixed it. Will share in 5 mins. It is one of those actions from the listeners cases. Separation to the command fixed it.

[oleg-shilo]

class navigate_to_line_async(sublime_plugin.WindowCommand):

    def run(self):

        def do():
            map_view = get_code_map_view()
            navigate_to_line(map_view , give_back_focus = not CodeMapListener.navigating)

        sublime.set_timeout_async(do, 10)

# =================================

def on_text_command(self, view, command_name, args):
    '''Process double-click on code map view'''

    double_click = (command_name == 'drag_select' and 'by' in args.keys() and args['by'] == 'words')

    if double_click:
        if ACTIVE and view.file_name() == code_map_file():
            win().run_command("navigate_to_line_async")
            return ("code_map_select_line", None)

[mg979]

It makes it a bit slower but it works.. Though I'm not sure it's better... The other problem is much bigger, though, still trying to find a solution, maybe almost there.

[oleg-shilo]

In my environment I didn't notice slowing down and it worked always. But I do feel like it is a work around but not a fix for the cause.

Anyway, as a side exercise I made this little marshaler that marshals a given routine into the sublime command and executes it asynchronously. It can be used instead of navigate_to_line_async and for any other heavy routines invoked from the listener:

# sample:
# marshaler.invoke(lambda: print('test'))

import uuid

class marshaler(sublime_plugin.WindowCommand):
    _actions = {}

    def invoke(action, delay=10):
        action_id = str(uuid.uuid4())
        marshaler._actions[action_id] = (action, delay)
        win().run_command("marshaler", {"action_id": action_id} )

    def run(self, **args):
        action_id = args['action_id']
        action, delay = marshaler._actions.pop(action_id)
        sublime.set_timeout_async(action, delay)

[mg979]

I'm almost done with my changes. I think I fixed that problem, at least I hope. A couple of things have changed(not dramatically). While I was there, I fixed a bug with navigation (error when starting from a region before the first map node) that was there since the beginning and I could finally eradicate. I included your fix from https://github.com/oleg-shilo/sublime-codemap/pull/14#issuecomment-334475502, but not that async command. I'll leave it to you if you want to incude it. I will make a commit, if for you it's ok I'll make a PR.