Proposal: abandon #303 and VR3

85 views
Skip to first unread message

Edward K. Ream

unread,
Sep 21, 2016, 4:45:44 PM9/21/16
to leo-editor
I'm having second thoughts about the wisdom of merging VR and VR2 into VR3, for the following reasons:

1. The code bases are significantly different, despite similarities.

2. Most people will strongly prefer one or the other.

3. There is no great need for the merger.  In fact, it's possible to enable both at once, and nothing awful will happen.

4. Keeping the plugins separate will allow independent development.

Unless I hear convincing reasons to do otherwise, I am going to close #303 and move the VR3 code to the attic.

Edward

lewis

unread,
Sep 21, 2016, 6:24:13 PM9/21/16
to leo-editor
I was under the impression (possibly mistaken) that vr3 was the future and that vr + vr2 were headed for the attic. So now would be a good time to summarise the actual feature sets or differentiating features of vr, vr2 and vr3. I think this will help in reviewing any convincing reasons.

Updating the plugins documentation and http://leoeditor.com/plugins.html#viewrendered-py to keep users aware of PyQt5 issues would be helpful.
Terry made a proposal to include python/Qt compatibility and maintenance status in the docstrings.

Re: 2. Most people will strongly prefer one or the other.
It should be quite clear to users which VR version is actively supported or parked in the attic

Re: 4. Keeping the plugins separate will allow independent development.
It should be clear and documented why different VR's are being kept alive for development.

Regards
Lewis

Edward K. Ream

unread,
Sep 22, 2016, 6:52:17 AM9/22/16
to leo-editor
​​
On Wed, Sep 21, 2016 at 5:24 PM, lewis <lewi...@operamail.com> wrote:

I was under the impression (possibly mistaken) that vr3 was the future and that vr + vr2 were headed for the attic.

​That was the original plan.  This proposal it an acknowledgement that this plan wasn't working.

The plain fact of the matter is that I have made more progress on improving vr in the last day or so than in the previous year.  Why?  Because I simply focused on vr.

Merging vr and vr2 is a hard project. Perhaps a new kind of Leo tool would help, but  I don't know what such a thing would be. It's in the back of my mind...
 
So now would be a good time to summarize the actual feature sets or differentiating features of vr, vr2 and vr3.

You can read the vr and vr2 docstrings to see the differences.  Better, simply run vr2 to see the new features it offers.

More importantly in practice, vr is based on a QtWebKitWidgets.QWebView (Qt5 terminology, per leoQt.py). vr2 is based on a QtWebKitWidgets.QWebView().  At present, vr2 does not work with PyQt5.  Substantial porting would be required.

Updating the plugins documentation and http://leoeditor.com/plugins.html#viewrendered-py to keep users aware of PyQt5 issues would be helpful.

​True.  vr2 now refuses to run (and tells why) when PyQt5 is in effect.
 
Terry made a proposal to include python/Qt compatibility and maintenance status in the docstrings.

​That's a separate issue.  In practice, such standards are difficult to maintain over time, even if somebody enforces them once.

Edward

Edward K. Ream

unread,
Sep 22, 2016, 5:19:31 PM9/22/16
to leo-editor
On Wednesday, September 21, 2016 at 3:45:44 PM UTC-5, Edward K. Ream wrote:

> I'm having second thoughts about the wisdom of merging VR and VR2 into VR3

As mentioned just now in a comment to #303, there is a much easier way to do the merge.  Just add some commands to the vr2 plugin, without using QWebView or any other part of the VR2 gui.

I should have thought of this earlier.  Today's workarounds of Qt5 troubles shows that QTextBrowser can do enough to emulate QWebView.

Indeed, given the problems with QWebView, it would seem natural to use QTextBrowser for the bigdash plugin as well.  Perhaps QWebView should be deprecated for the time being.

Edward

lewis

unread,
Sep 22, 2016, 6:22:22 PM9/22/16
to leo-editor
I just ran last commit 3f0d2ce, running vr2 and PyQt5 to test the new warnings:

Leo Log Window
Leo 5.4-devel, build 20160722143100, Fri, Jul 22, 2016  2:31:00 PM
Git repo info: branch = master, commit = 3f0d2ce1acd8
Python 3.5.2, PyQt version 5.7.0
Windows 10 AMD64 (build 10.0.14393) SP0
The viewrendered2 plugin is not compatible with PyQt5
loadOnePlugin: can not load enabled plugin: leo.plugins.viewrendered2
reading: N:\leo\DServe_Eff_Filters.leo
read outline in 0.05 seconds
viewrendered plugin loaded.


Leo successfully reports the that viewrendered2 doesn't get on well with PyQt5, and proceeds to load viewrendered plugin. All the unpleasant errors are gone.
This is very neat!

Thanks
Lewis



On Thursday, 22 September 2016 20:52:17 UTC+10, Edward K. Ream wrote:
​​
The plain fact of the matter is that I have made more progress on improving vr in the last day or so than in the previous year.  Why?  Because I simply focused on vr.
[snip]

Edward K. Ream

unread,
Sep 23, 2016, 8:10:03 AM9/23/16
to leo-editor
On Thu, Sep 22, 2016 at 5:22 PM, lewis <lewi...@operamail.com> wrote:

Leo successfully reports the that viewrendered2 doesn't get on well with PyQt5, and proceeds to load viewrendered plugin. All the unpleasant errors are gone.
This is very neat!

​This is more of a bug than a feature ;-)  It's hard/impossible to disable a plugin if it's in leoSettings.leo or myLeoSettings.leo.

In fact, vr and vr2 can both be enabled.  They will handle the same events, and the first one that is called will suppress the second.

And it's hard not to enable plugins in leoSettings.leo​: otherwise people wouldn't get any default plugins.

I've been thinking about this lately.  It's time for an enhancement request.  We would like a way of disabling plugins on a per-file basis, maybe by disbling event handling for a plugin (with the possible exception of shutdown/window-close events.) Enabling vr2 in a local file would be a good test.  We want vr enabled in most files, but vr2 in that local file.

Edward

Edward K. Ream

unread,
Sep 23, 2016, 11:23:29 AM9/23/16
to leo-editor
On Friday, September 23, 2016 at 7:10:03 AM UTC-5, Edward K. Ream wrote:
On Thu, Sep 22, 2016 at 5:22 PM, lewis <lewi...@operamail.com> wrote:

Leo successfully reports the that viewrendered2 doesn't get on well with PyQt5, and proceeds to load viewrendered plugin. All the unpleasant errors are gone.
This is very neat!

​This is more of a bug than a feature ;-)  It's hard/impossible to disable a plugin if it's in leoSettings.leo or myLeoSettings.leo.

I've just created #312 to address this issue.  I'd like to fix this for Leo 5.4b1.

EKR

lewis

unread,
Sep 23, 2016, 8:13:37 PM9/23/16
to leo-editor
Running commit 6e9d291, I ran vr and leo flashes the vr pane for a fraction of a second then leo closes completely.

Here is log after loading:
Leo Log Window
Leo 5.4-devel, build 20160722143100, Fri, Jul 22, 2016  2:31:00 PM
Git repo info: branch = master, commit = 6e9d2918348d

Python 3.5.2, PyQt version 5.7.0
Windows 10 AMD64 (build 10.0.14393) SP0
The viewrendered2 plugin is not compatible with PyQt5
loadOnePlugin: can not load enabled plugin: leo.plugins.viewrendered2
reading: N:\leo\DServe_Eff_Filters.leo
read outline in 0.05 seconds


And the console trace:
[snip]
reading settings in N:\leo\DServe_Eff_Filters.leo

The viewrendered2 plugin is not compatible with PyQt5
loadOnePlugin: can not load enabled plugin: leo.plugins.viewrendered2
Traceback (most recent call last):
  File "N:\git\leo-editor\leo\plugins\viewrendered2.py", line 823, in render_delegate
    self.render()
  File "N:\git\leo-editor\leo\plugins\viewrendered2.py", line 833, in render
    self.render_helper()
  File "N:\git\leo-editor\leo\plugins\viewrendered2.py", line 844, in render_helper
    self.html = html = self.to_html(p)
  File "N:\git\leo-editor\leo\plugins\viewrendered2.py", line 876, in to_html
    ps = mf.scrollBarValue(QtCore.Qt.Vertical)
AttributeError: 'QWebEngineView' object has no attribute 'scrollBarValue'

[end]

Regards
Lewis

Edward K. Ream

unread,
Sep 24, 2016, 5:15:07 AM9/24/16
to leo-editor
On Fri, Sep 23, 2016 at 7:13 PM, lewis <lewi...@operamail.com> wrote:

Running commit 6e9d291, I ran vr and leo flashes the vr pane for a fraction of a second then leo closes completely.

​Thanks for this report.  vr2 is known not to work with PyQt5.​
 

​This is a strange one: the log says:

​​
​​loadOnePlugin: can not load enabled plugin: leo.plugins.viewrendered2
The viewrendered2 plugin is not compatible with PyQt5

and yet the crash happens in vr2. I'll fix this before b1.  I'll say more about vr+vr2 in another thread.

Edward

Edward K. Ream

unread,
Sep 24, 2016, 6:58:28 AM9/24/16
to leo-editor
On Sat, Sep 24, 2016 at 4:15 AM, Edward K. Ream <edre...@gmail.com> wrote:
On Fri, Sep 23, 2016 at 7:13 PM, lewis <lewi...@operamail.com> wrote:

​​
Running commit 6e9d291, I ran vr and leo flashes the vr pane for a fraction of a second then leo closes completely.

​Fixed at 31b7487​ by defining vr2 commands only if the plugin actually loaded correctly.  This can be done elegantly by putting the following in the organizer node for all commands:

if import_ok:
    # Define the commands only if this plugin is active.
    @others

Edward

lewis

unread,
Sep 24, 2016, 7:37:32 AM9/24/16
to leo-editor
Thanks for the fix, these vr/vr2 plugins are now significantly more robust.

With viewrendered2 only enabled: Leo reports 'The viewrendered2 plugin is not compatible with PyQt5'
If you run vr commands, Leo remains stable and does not attempt or perform any rendering.

With viewrendered and viewrendered2 both enabled: Leo reports 'The viewrendered2 plugin is not compatible with PyQt5'
If you run vr commands, Leo remains stable, and apparently switches to using viewrendered to render in the VR pane.

May I suggest that Leo reports to the log pane 'switching to viewrendered' where applicable?

Regards
Lewis

Edward K. Ream

unread,
Sep 24, 2016, 10:52:47 AM9/24/16
to leo-editor
On Sat, Sep 24, 2016 at 6:37 AM, lewis <lewi...@operamail.com> wrote:
Thanks for the fix, these vr/vr2 plugins are now significantly more robust.

​Thanks for the confirmation.


​> ​
May I suggest that Leo reports to the log pane 'switching to viewrendered' where applicable?

​Imo, that would quickly become annoying.

EKR
Reply all
Reply to author
Forward
0 new messages