Test for and follow tab changes in multipane mode

[politas]

ISSUE TYPE

• Bug fix for Status bar should reflect active pane in multipane viewmode #1880

RUNTIME ENVIRONMENT

• Operating system and version: Manjaro Linux
• Terminal emulator and version: xfce4-terminal 0.8.9.2
• Python version: 3.8.3 (default, May 17 2020, 18:15:42) [GCC 10.1.0]
• Ranger version/commit: ranger-master
• Locale: en_AU.UTF-8

CHECKLIST

• The CONTRIBUTING document has been read [REQUIRED]
• All changes follow the code style [REQUIRED]
• All new and existing tests pass [REQUIRED]

DESCRIPTION

Bound tab.change and setopt.viewmode signals to StatusBar widget request_redraw
Added a new test to def draw() that enforces current column to match browser main_column

MOTIVATION AND CONTEXT

Solves the bug of the Status bar getting locked to an old directory when switching tabs in multipane mode and on returning to miller mode.

TESTING

Ran all existing tests successfully.
This only affects the statusbar when multipane mode is used.

[politas]

The statusbar is still not updating when reverting from multipane to miller mode. I would apreciate some advice on fixing that; otherwise, I will dive into the issue when I next have free time.

[toonn]

I think the tab.layoutchange signal might be more appropriate. For the viewmode you should check whether setopt.viewmode is sent.

[politas]

Doesn't tab.layoutchange only trigger when the number of tabs changes?

Ah, setop.viewmode is another signal.

I'm thinking we need another set of tests on viewmode[0] to check that self.column == self.fm.ui.browser.column to fix the return to miller mode? That should be triggered by setop.viewmode, I see.

[politas]

Status bar is now updating correctly on all mode changes and tab changes by my testing.

[toonn]

tab.change isn't triggered when closing a tab I think? While tab.layoutchange is.

[politas]

It does pick up the change. Closing the open tab forces a tab.change, surely?

[toonn]

Some quick grepping seems to indicate no though? I can't find the signal_emit for it anyway.

[politas]

Ok, it will now catch tab.layoutchange as well.

[toonn]

Hmm, is there a way to coalesce these signals? You could be causing quite a lot of redrawn the way it is right now.

[politas]

That's why I was hesitant to include tab.layoutchange when it didn't seem necessary. I really haven't a clue about that at this time. I'm not really grokking the signals.

[politas]

How much trouble is adding a new signal? Would it be better to add a tab.redraw signal to the places that trigger tab.change and tab.layoutchange?

[toonn]

I really think layoutchange is supposed to be that signal. Which occurences of tab.change aren't covered by tab.layoutchange?

[politas]

Does simply switching the active tab in multipane mode trigger a tab.layoutchange if the only difference is which column is the main_column?

[politas]

In ViewMultipane, tab.layoutchange triggers a full rebuild, while tab.change is only a redraw. That implies tab.change signals being triggered separately to tab.layoutchange

[politas]

Binding only to tab.change leaves out the case where a tab exists and is in the layout, but hasn't been opened. (When you open in multitab mode with remembered tabs and only the current tab is populated initially while setting a multi-pane layout including spaces for each tab, and then hit "tab" to switch tabs.) The pre-existing tab (shown in the tab list) is then populated as you switch to it, but the status bar is not updated. The status bar will start updating as soon as you switch to an already-visible tab.

I'm not sure exactly which actions are being called in order to make that happen. We should probably trigger a tab.change signal in that case.

[politas]

Or maybe, if we open in multi-pane mode with remembered tabs, we should actually make all the tabs visible?
Edit: raised as #2063

[toonn]

I think tab.change should only be sent if the directory in a tab changes, this includes going from no directory to a directory (new tab). It means a tab changes, not that ranger's view of tabs changes. This leaves several cases where the signal would not be triggered but a redraw is warranted.

I agree that all the tabs should be visible when opening with remembered tabs in multipane. (This does have performance implications of course, we normally load tabs lazily to avoid exactly this.) A config option limiting the number of open panes in the multipane mode seems desirable.

[politas]

Checking with a debugger, with the code as it is in this PR (binding to both tab.change and tab.layoutchange), the actual redraw only happens once, no matter how many signals have been received. This would seem to be exactly what we want to happen. Leaving out the binding to tab.change means it fails to be triggered by the tab_restore action

Actions	Signals emitted
tab_open	tab.change, tab.layoutchange
tab_close	tab.layoutchange
tab_shift	tab.layoutchange
tab_restore	tab.change

[toonn]

I think tab_restore should emit tab.layoutchange as well, since it's much like opening a new tab. That way you'd only need to handle the one signal.

[politas]

I suppose the reason it doesn't is that the layout of the multipane doesn't change, merely the contents of one pane. What's wrong with binding to both signals? I'd rather not mess with the signal emits, since they are used elsewhere.

[toonn]

Looks to me like it recreates a tab? That adds a tab no? Which changes the layout, just like tab_open does.

[politas]

Seems to me that redesigning the signalling of tab adjustments is outside the scope of this PR, which is tightly focused on getting the status bar to pay attention to those adjustments when they happen. Can we raise a different issue for a review of tab signalling?

[toonn]

A different PR is fine. I consider it a bug that something which affects the tabs the same way tab_open does would not raise the same signals.

[politas]

tab_restore() immediately leads to a tab_open(), triggering a tab.layoutchange anyway. I can't actually find any action that triggers a tab.change without also triggering a tab.layoutchange

[toonn]

Ah, perfect, thanks for looking into it. Then we can drop listening to tab.change I guess.

[politas]

I'd really love to have this merged. The messed up status bar is a real nuisance.

[toonn]

Thanks for the ping, I'd lost track of this one.

[toonn]

Hmm, I was looking into this to merge it but I can't reproduce the problem. I open a couple tabs in multipane viewmode and select different files in each of them. The statusbar changes permissions and date when changing tabs.

[politas]

It only goes wrong when you change to a different directory while in multipane mode. And then, once it's disconnected, even switching back to single pane mode is still messed up. Once it disconnects, the status bar is locked to that specific directory.

[toonn]

Disconnects? So it only works for networked directories?

[politas]

I mean the status bar is disconnected from the active pane. It is not responsive to the current selection. I don't think there is any distinction between folders that are part of networked directories, is there? I don't know why ranger would care whether a file system is local or networked.

[politas]

Here's a demonstration

[toonn]

Thank you for the video, very clear explanation!

LGTM 👍