ekr-unl2-new branch: progress report

31 views
Skip to first unread message

Edward K. Ream

unread,
Feb 13, 2022, 7:58:31 AM2/13/22
to leo-editor
Please test this branch and report any problems.

- PR #2409 summarizes the present work.
- Thomas has filed #2410, which applies to all present code.

Discussion

The following remarks will be pre-writing for the underlying issue, #2403.

The new code removes all kwargs from p.get_UNL, thereby using a single, simplified, format for UNLs. This new format has the form of a '-->' separated list of headlines, with an optional trailing line number part of the form '::n'. A negative n denotes a global offset (in an external file). A positive n denotes an offset within a node.

Note that the line number part starts with '::'. In old UNLs, headlines could be followed by ':' and zero to 4 other numbers. The change from ':' to '::' allows code to distinguish between old and new UNLs. In fact, the new code ignores old UNLs.

The old UNL format was (slightly) more precise than the new. The old format could disambiguate cloned siblings. The new format can not. In practice, this ambiguity will almost never matter. I am about to revise the backlink plugin so that it uses archived positions instead of UNLs. This change might cause problems for users of the backlink plugin, but imo that's not a big deal.

Summary

p.get_UNL now returns new UNL's in a single, intuitive format. All kwargs to p.get_UNL are gone, which collapses the complexity of all UNL-related code.

New UNLs can not distinguish cloned sibling nodes. In practice, cloned siblings are rare, and it should never matter what cloned sibling gets selected.

I shall soon revise the backlink plugin so that it uses archived positions instead of UNLs. That change may break existing back links.

Edward

Edward K. Ream

unread,
Feb 13, 2022, 9:46:41 AM2/13/22
to leo-editor
On Sunday, February 13, 2022 at 6:58:31 AM UTC-6 Edward K. Ream wrote:

> I shall soon revise the backlink plugin so that it uses archived positions instead of UNLs.

After another code review I have decided to leave the backlink plugin alone. The code is too complex to change safely.

The recent changes to g.findUNL will break saved links with the old UNL format. Happily, the plugin reports broken links. The simplest approach would be to let the users recreate the links. All new links will use new UNL's.

Edward

tbp1...@gmail.com

unread,
Feb 13, 2022, 10:25:29 AM2/13/22
to leo-editor
This work is timely from my point of view, since the project I am working on will be using them.  In the original code, I have observed that g.handleUnl() didn't work right when there are index numbers in the UNL.  Since there will now be no option for omitting them in p.get_UNL(), then g.handleUnl() will need to be robust against their presence or absence.  Also, those index numbers are likely to change because of normal editing even when the target node has not been moved.  So g.handleUnl() should at least find the right node anyway.

[Aside] I don 't think that handleUnl is a good name here.  It doesn't say what will happen: "handling" could mean anything.  I'd like to see it deprecated in favor of a new method, say gotoUnl.

Edward K. Ream

unread,
Feb 13, 2022, 3:56:41 PM2/13/22
to leo-editor
On Sunday, February 13, 2022 at 8:46:41 AM UTC-6 Edward K. Ream wrote:

> The recent changes to g.findUNL will break saved links with the old UNL format.

Not anymore. g.findUNL now contains the convert_unl_list helper function that converts old-style UNL's to new-style UNL's. This should allow the backlink plugin to continue to use old-style links.

Edward

Edward K. Ream

unread,
Feb 13, 2022, 4:04:39 PM2/13/22
to leo-editor
On Sunday, February 13, 2022 at 9:25:29 AM UTC-6 tbp1...@gmail.com wrote:

This work is timely from my point of view, since the project I am working on will be using them.  In the original code, I have observed that g.handleUnl() didn't work right when there are index numbers in the UNL.  Since there will now be no option for omitting them in p.get_UNL(), then g.handleUnl() will need to be robust against their presence or absence. 

g.handleUnl  does not need to change. g.handleUnl knows nothing about index numbers, and never has.

Also, those index numbers are likely to change because of normal editing even when the target node has not been moved.  So g.handleUnl() should at least find the right node anyway.

It is g.findUNL that needs to be robust, and the new version of g.findUNL is :-)  In fact, p.get_UNL only generates a line number for the last element of the unl list. All other elements are simply headlines. The new  convert_unl_list helper appears to handle old UNLs properly.  We shall see.

I shall soon merge the ekr-unl2-new branch into the ekr-unl2 branch, and then the ekr-unl2 branch into devel. It's time for wider testing of the new code. We aren't going back to the old code. It should be safe enough to handle any new problems in a new PR.

[Aside] I don 't think that handleUnl is a good name here.  It doesn't say what will happen: "handling" could mean anything.  I'd like to see it deprecated in favor of a new method, say gotoUnl.

A agree the name isn't great, but imo the name is not important enough to change.

Edward

tbp1...@gmail.com

unread,
Feb 13, 2022, 5:29:17 PM2/13/22
to leo-editor
On Sunday, February 13, 2022 at 4:04:39 PM UTC-5 Edward K. Ream wrote:
On Sunday, February 13, 2022 at 9:25:29 AM UTC-6 tbp1...@gmail.com wrote:
g.handleUnl  does not need to change. g.handleUnl knows nothing about index numbers, and never has.

Except that empirically, when the headline steps had indexes, handleUnl() didn't navigate right, at least for the few tests I did yesterday.

Also, those index numbers are likely to change because of normal editing even when the target node has not been moved.  So g.handleUnl() should at least find the right node anyway.
 
[Aside] I don 't think that handleUnl is a good name here.  It doesn't say what will happen: "handling" could mean anything.  I'd like to see it deprecated in favor of a new method, say gotoUnl.

A agree the name isn't great, but imo the name is not important enough to change.

Not change, just assign the new name to the old so they both refer to the same method.  Over time, people who care (like me, anyway) could start to use the new name.
 

Edward K. Ream

unread,
Feb 13, 2022, 8:34:18 PM2/13/22
to leo-editor
On Sun, Feb 13, 2022 at 4:29 PM tbp1...@gmail.com <tbp1...@gmail.com> wrote:

>> g.handleUnl  does not need to change. g.handleUnl knows nothing about index numbers, and never has.

> Except that empirically, when the headline steps had indexes, handleUnl() didn't navigate right, at least for the few tests I did yesterday.

g.findUNL does all the real work. Please try the latest code in devel.

Edward

tbp1...@gmail.com

unread,
Feb 14, 2022, 12:21:23 AM2/14/22
to leo-editor
With the latest version of devel - 70d0da6013 - c.p.get_UNL() produces a UNL with no line index (not saying that's wrong). CNTL-click on that UNL pasted into a test node usually does not go to the right node in the target outline (with or without replacing spaces by "%20", and with or without replacing ">" by "%3E").

Edward K. Ream

unread,
Feb 14, 2022, 7:09:32 AM2/14/22
to leo-editor
On Sun, Feb 13, 2022 at 11:21 PM tbp1...@gmail.com <tbp1...@gmail.com> wrote:
With the latest version of devel - 70d0da6013 - c.p.get_UNL() produces a UNL with no line index (not saying that's wrong).

That is the intended behavior.  Callers of p.get_UNL should add the line number ('::' plus the line number) to create clickable links.  A cff on "nodeLink" will show you the pattern.

CNTL-click on that UNL pasted into a test node usually does not go to the right node in the target outline (with or without replacing spaces by "%20", and with or without replacing ">" by "%3E").

That's a bug. I have just created #2314 for this. Should be easy to fix.

Edward

Edward K. Ream

unread,
Feb 14, 2022, 8:48:00 AM2/14/22
to leo-editor
On Monday, February 14, 2022 at 6:09:32 AM UTC-6 Edward K. Ream wrote:

CNTL-click on that UNL pasted into a test node usually does not go to the right node in the target outline (with or without replacing spaces by "%20", and with or without replacing ">" by "%3E").

That's a bug. I have just created #2314 for this. Should be easy to fix.w

Please test  the ekr-unl3 branch. It contains the proposed fix.

Edward

tbp1...@gmail.com

unread,
Feb 14, 2022, 10:01:37 AM2/14/22
to leo-editor
CTRL-clicking succeeded on the UNLs that previously failed.  This includes one whose terminal step included a section name (<< ... >>).  The tests included both relative (within-outline) and absolute (between outline) UNLs.  When the UNL included a trailing index, the cursor was placed on correct line in the target.  Replacing spaces with "%20" or ">" with "%3E" did not change the behavior.

Leo 6.6b2-devel, ekr-unl3 branch, build 191fd7560d
2022-02-14 07:26:41 -0600
Python 3.9.9, PyQt version 5.15.2
Windows 10 AMD64 (build 10.0.19043) 


Edward K. Ream

unread,
Feb 14, 2022, 10:49:56 AM2/14/22
to leo-editor
On Mon, Feb 14, 2022 at 9:01 AM tbp1...@gmail.com <tbp1...@gmail.com> wrote:
CTRL-clicking succeeded on the UNLs that previously failed.  This includes one whose terminal step included a section name (<< ... >>).  The tests included both relative (within-outline) and absolute (between outline) UNLs.  When the UNL included a trailing index, the cursor was placed on correct line in the target.  Replacing spaces with "%20" or ">" with "%3E" did not change the behavior.

Thanks for this report! I have just merged the ekr-unl3 branch into devel.

Edward
Reply all
Reply to author
Forward
0 new messages