PR for #3892: rewrite the VR plugin

[edreamleo]

See #3892.

This PR does not change the VR3 plugin in any way, but may serve as a model to clean the VR3 plugin.

Commands and features

This PR adds the following commands, giving Leo about 80% of Trillium's features:

• vr-fully-expand: fully expand the VR pane so it covers the body pane.
• vr-restore-body: undo the effect of vr-fully-expand.
• vr-toggle-visibility: Same as legacy vr-toggle command.
• vr-toggle-keep-open: Toggles whether the VR pane always stays open.
  By default, the VR pane opens only if the pane shows rendered text.
• Add log messages (in red) when the status of the VR pane changes.
  These messages are essential to avoid confusion.
• Suppress docutils errors from appearing in the console.
  The errors appearing in the VR pane suffice.

Bugs fixed

• Fix serious bugs in vr.get_kind.
• Update nodes containing @language rest immediately.

Code changes

• Remove vr.adjust_layout and vr.restore_layout.
  These methods create unwanted and unnecessary state.
  The obvious placement of the VR pane suffices.
• Delete the VR controller only when closing its outline (commander).
• Replace pc by self everywhere.

Extras

• FreeLayoutController.get_main_splitter always returns the NestedSplitter whose name is main_splitter.
  Imo, this is the only return value that is logically invariant.
• Improve error message in leoTokens.py.
• Improve SqlitePickleShare.__repr__

[edreamleo]

@boltex @tbpassin This PR is ready for review. It was surprisingly easy.

[edreamleo]

@tbpassin I think your contribution is essential. I've just sent you a zoom invite.

[tbpassin]

Now I sometimes get this error:

Traceback (most recent call last):
  File "c:\Tom\git\leo-editor\leo\plugins\nested_splitter.py", line 113, in <lambda>
    act.triggered.connect(lambda checked: func())
                                          ^^^^^^
  File "c:\Tom\git\leo-editor\leo\plugins\nested_splitter.py", line 260, in rotate_only_this
    splitter.rotateOne(index)
  File "c:\Tom\git\leo-editor\leo\plugins\nested_splitter.py", line 864, in rotateOne
    psp.setSizes(sizes)
TypeError: index 0 has type 'float' but 'int' is expected

Leo 6.7.9-devel, ekr-3892-vr-pane branch, build 82b0f5debf
2024-05-11 08:45:46 -0500
Python 3.12.3, PyQt version 6.6.2
Windows 10 AMD64 (build 10.0.19045) SP0

[edreamleo]

@tbpassin Thanks for this report. I half remember a similar problems recently. The solution should be straightforward now that Leo requires Qt6 .

[tbpassin]

I'm still reviewing the changes but I want to point out something that I've just realized. If I don't enable VR3 in the settings, my toggle-in-body script works normally anyway. Once I've run it, all the VR3 commands become available - because they get created when the module first gets imported.

But the not-enabled VR3 pane is not available to the splitter context menu. Since the "provider" init code doesn't get run on import, you can't create a free-floating window for it or open a new pane containing it. It isn't listed in the Plugins menu. But all the context menu layout options still work because they work on the panes that VR3 is embedded in, not on the plugin itself. I can still toggle it on and off in a tab in the Log frame. I can still swap it for the body editor view.

So what if I can't create a free-floating window with VR3? So what if I can't create a new pane with a VR3 view in it? I basically don't want to do these things. (I think someone wrote that they do like to open VR3 in a freed-floating window, I forget who. Not sure what we should do about them). Maybe we can get rid of the provider class and its init code altogether and never need to "enable" the VR/VR3 plugins!

We would lose the ability to create a new pane and stick VR/VR3 into it. That would affect how the F11 help messages get displayed. We'd have to think about that. The docstring would no longer be available via the Plugins menu and we'd have to work out how to make it easy for a user to find and view it. There's probably a way. We'd gain freedom from all this pesky layout stuff. Leave layout to the containing frames, not our plugins.

[tbpassin]

[1] free_layout.py. line 197 -
We tacitly assume that this splitter contains the body pane.
The body could have been moved into a free-floating window. If so, will this assumption still hold? If not, will there be bad consequences?

==========================
[2] viewrendered.py - line 354:
Handle a close event in the Leo *outline*, not the VR pane.

This docstring is ambiguous to me. Is the close event in the outline? What about a close event in the VR pane? What's the difference? Or are we supposed to handle the event in the outline? Please revise this text to be more clear about what the function will do.

=============================
[3] viewrendered.py, lines 414-415:

        if splitter:
            splitter.add_adjacent(vr, 'bodyFrame', 'right-of')

We can only open in a new pane to right of the body. I'm not sure we want this. Also, in trying this out, the command vr did not display VR. Instead, it only focused the body frame.

=================================

[4] viewrendered.py, lines 766ff -

        self.setObjectName('viewrendered_pane')
        self.setLayout(QtWidgets.QVBoxLayout())
        self.layout().setContentsMargins(0, 0, 0, 0)

What if there already is a pane with the same name?

========================================
[5] viewrendered.py, lines 782ff -
def change_size()

I question whether this is a good idea. The user can change panel sizes by dragging the splitter bar, and it's much easier to get what one wants.

In VR3, instead of this command I just use the QWebEngine's method to set a zoom factor. This changes the rendered size but doesn't change the splitters.

================================
[6] viewrendered.py, lines 817ff -
def restore_body()
I hated this method. It's very annoying to have the splitter bar jump all over the place. If the VR pane had truly perfectly covered the body editor, and then this command made the splitter bar to slide perfectly over to fully hide the VR pane, ti would have been marginally OK. But it rarely seemed to work right.

================================
[7] viewrendered.py, line 891 -
"""Create a QWebView or a QTextBrowser."""

Update QWebView for QT6's QWebEngineView

=========================================
[8] viewrendered.py, line 942 -

      if self.length != p.v.b
             self.length = len(p.b)  # Suppress updates until next change.

I think this should be

      if self.length != len(p.v.b):
             self.length = len(p.v.b)  # Suppress updates until next change.

======================================
[9] viewrendered.py, lines 1577 ff -

            if p == p0:
                # Careful: never use c.target_language as a default.
                return g.findFirstValidAtLanguageDirective(p.b)

What do we do if p != p0?

=====================
[10] viewrendered.py, lines 1594 ff -

           if language:
                if language in ('md', 'markdown'):
                    return language if got_markdown else None
                if language in ('rest', 'rst'):
                    return language if got_docutils else None
                if language in self.dispatch_dict:
                    return language

Since I noticed that VR is allowing asciidoc, there should be a got_asciidoc flag and it should be tested for.

But note that there is asciidoc, and also asciidoc3, which are not quite the same. With VR3, there is also the possibility of using an external ruby asciidoctor processor, so there is a lot of complication in figuring just which one to use if there are more than one, and they get launched differently.

The ruby processor can be important because it is the one that can render Plantuml diagrams and probably other kinds as well. But it's fearfully slow for anything complicated. And even for a simple node, you have to spin up the external processor for each update.

==================================
[11] As a general remark, as of this rev I still can't get VR to open reliably. Sometimes (most of the time) it still opens that external blank window which doesn't respond to any vr- commands. At least I now know how to close it without closing the entire Leo app.

In Windows the tasl bar will show you all the Python processes associated with each Leo outline (I didn't know they were separate processes!). The one with the blank dead window will show an unnamed sibling of the outline process. That's the one. You can kill it with the "End Task" button without messing up the Leo session itself.

[edreamleo]

@tbpassin Thanks for your comments. Some comments:

Point 1: The free_layout plugin can change the relationships between Qt widgets in impossible-to-predict ways. Why is this fact VR's problem?

Point 2: The docstring is correct and clear. Closing the VR pane is different from closing the outline containing the VR pane. In the former case, the VR plugin hides the pane. In the latter, the VR plugin destroys the VR controller.

Point 3: The VR plugin creates the VR pane as part of the body pane. The VR3 plugin can do as it likes. The VR plugin is mine now, and I'm not going to argue about such matters.

Point 4: This is (or should be) free_layout's problem.

Point 5: The commands that change the size of the VR pane have been around a long time. I am going to leave them as they are for compatibility.

Point 6: Opening and closing the VR pane doesn't bother me at all. The vr-toggle-keep-open command will calm things down.

Point 7: It appears that vr.create_base_text_widget creates only a QTextBrowser. I'll update the docstring.

Point 8: Good catch.

Point 9: If p != p0 the code falls through. The "missing" else is a consequence of pylint's default checker rules.

Point 10: These complications should be left for another PR.

Point 11: I'm not happy that you are still having problems, but I doubt that this PR is to blame.

Finally, I would like a simpler alternative to the free_layout plugin. I have some ideas, but they need more work.

[edreamleo]

@tbpassin I've fixed the bug you found.

Please approve this PR. It's already too big. We can fix all remaining problems in smaller PRs.

[edreamleo]

@tbpassin The VR panes doesn't play well with several options in the Easter Egg.

I'll attempt to fix these problems before marking the PR ready for review.

[edreamleo]

@tbpassin Rev 8ede3fc appears to make VR work with free_layout.

[tbpassin]

Point 3: The VR plugin creates the VR pane as part of the body pane. The VR3 plugin can do as it likes. The VR plugin is mine now, and I'm not going to argue about such matters.

I'm not trying to argue or tell you what you should do. I just wanted to raise the question of whether opening another pane to the right is the only action you would ever want to do.

Point 2: The docstring is correct and clear. Closing the VR pane is different from closing the outline containing the VR pane. In the former case, the VR plugin hides the pane. In the latter, the VR plugin destroys the VR controller.

It was a little ambiguous to me because I could read it as instructions to a programmer, or a description of what should or should not happen. I would have preferred something like this:

Destroy the VR pane and controller after a close event in the Leo *outline*.
 
The VR pane will handle a close event in its own frame elsewhere.

Point 9: If p != p0 the code falls through. The "missing" else is a consequence of pylint's default checker rules.

Well, yes, I know that. I meant that I wasn't sure if there was intended to be a missing else clause or not. I just wanted you to check for that.

Point 10: These complications should be left for another PR.

I completely agree. I just wanted to get you thinking a bit about it for the future.

Point 11: I'm not happy that you are still having problems, but I doubt that this PR is to blame.

I just tried it in a Linux VR. When I issued the command vr-toggle what happened is that the body got focus but no VR pane was to be seen. Looking closely, I saw the VR pane get created but then the splitter bar slid instantly to the far right and this hid the new VR pane. There was no handle for me to grab to slide over and reveal the VR pane. I saw this behavior - seeming to only bring focus to the body - on Windows when I didn't have the other anomalous behavior but it happened too fast for me to notice the new pane being created.

[tbpassin]

@tbpassin I've fixed the bug you found.

Please approve this PR. It's already too big. We can fix all remaining problems in smaller PRs.

I'll be happy to but Github isn't offering me a control to do so. Once I posted my review that control is not longer being presented.

[edreamleo]

@tbpassin OK. I'll merge this PR. Let's continue this discussion!

[tbpassin]

I just merged your latest changes, and on Linux, vr-toggle does that thing where the VR pane is created but hidden. After that issuing vr-show does display a normal-sized VR pane to the right of the body pane.

However, after closing the Leo window I got this error message, and it took a long time for the shutdown to complete:

QFontDatabase: Must construct a QGuiApplication before accessing QFontDatabase

Issuing vr-show first acted just like vr-toggle.

Leo 6.7.9-devel, ekr-3892-vr-pane branch, build 8ede3fc77e
2024-05-14 05:12:23 -0500

[edreamleo]

@tbpassin Thanks for your continued testing. The PR changed the shutdown logic to accommodate the free_layout plugin.

As I mentioned in the ENB, I'm going to experiment with bypassing free_layout entirely.

[edreamleo]

@tbpassin #3909 continues development on the rewritten VR plugin. Let's continue the discussion there.