ENB: About Leo's outline redraw code

[Edward K. Ream]

In this Engineering Notebook post I'll discuss the the code that draws Leo's outline pane. This post will explain why this code is complex and how it might possibly be improved.

Present status

1. Leo's tree code never drops data. This is crucial! It's all too easy to lose data when switching from one node to another.

2. Leo's tree code has good-to-excellent performance when few nodes are visible. In such situations, moving nodes can be done very quickly. The present code will slow significantly when large numbers of nodes are (potentially) visible.

3. The interface between Leo's code is minimal. Leo's core code can cause a full redraw (of only the visible tree nodes) merely by calling c.redraw(). The code can also schedule a redraw at idle time, but this is typically not needed.

Complicating factors

A major motivation for this post was "pyzo envy". Pyzo's tree drawing code, for example the PyzoSourceStructure class, is much simpler than Leo's code in qt_tree.py. Why is this so?

#1585 lists the features that complicate Leo's code base. I keep adding to the list! For this discussion, the following factors are most important:

1. The outline must support clones. Adding, moving or deleting a node may cause arbitrarily many changes to visible outline nodes!

2. Leonine scripts can add, move or delete nodes. The present code uses lockouts to ensure that scripting changes don't cause unwanted gui events.

3. Leo's core is (almost) completely separate from Leo's gui code.

Because of these complicating factors, #1576 seems moot. I have closed it.

Alternatives and trade offs

The present redraw code takes time proportional the number of potentially visible outline nodes. In other words, the code draws all visible nodes regardless of whether they are actually scrolled into view. This makes scrolling the outline pane trivial.

It would be possible to draw only the actually visible outline nodes. This would speed the redraw code at the expense of extra code complexity, especially when scrolling the outline pane.

At present, Leo completely redraws all (potentially) visible nodes when redrawing the screen. So the Qt code suffers a (usually mild) form of tree thrashing, the continual allocation/deallocation of Qt tree nodes. There are two possible ways to reduce this thrashing:

1. Leo could allocate Qt nodes for all outline nodes, visible or not, just once. Alas, because of clones, inserting, deleting and (especially) moving nodes can affect arbitrarily many other nodes! The code to do this would be difficult and error prone. Early versions of Leo used this technique. Back then (ca. 1990), the performance hit was substantial.

2. Leo could use difflib to minimize the amount of Qt nodes to be inserted or deleted. See #1068. This post discusses this idea in detail. Preliminary code already exists in leo/core/leoFastRedraw.py.

Note: screen thrashing would be unimportant if only actually visible nodes were allocated.

Summary

It's easy for anyone, including myself, to get confused about the code and how it might (or might not) be improved. This post lists what I think are the most important considerations.

The present code is reasonable and rock solid. There is no great need to revise it, and I have no plans to do so. Having said that, it's fine with me if someone wants to attempt improvements. Just make sure that the code remains rock solid. I'll be happy to discuss changes in more detail if you like.

Comments are welcome, but please do so in a separate thread. I want this post to remain unchanged.

Edward

[Edward K. Ream]

On Thursday, April 30, 2020 at 11:52:33 AM UTC-5, Edward K. Ream wrote:

> In this Engineering Notebook post I'll discuss the the code that draws Leo's outline pane...

> Comments are welcome, but please do so in a separate thread.

Never mind. I have unlocked this thread and unpinned it from appearing at the top of the page. All comments are now welcome.

Here are some further thoughts:

1. As I said in the original post, studying pyzo's code is unlikely to be useful. Leo's code base is just too different from Leo's. Instead, I recommend that Leo's devs should focus on design issues.

2. I agree with the comments made in #1585. Changes to Leo's code base must be made carefully and incrementally, ideally backed by existing or new unit tests.

For example, I have been dithering about #325. This issue is a good example of the risks and benefits of simplifying code. Imo, it's unbearably ugly to use per-file definitions of @cmd decorator instead of a single, global @g.command decorator. However, a cff shows that @cmd defines roughly 300 commands in Leo's core. Changing these commands must be done carefully and incrementally. This I intend to do.

3. Improving Leo's redraw code only makes sense if it significantly improved overall performance. That's not true now, and it would only become true if Leo were to support truly huge outlines. Imo, drawing only the actually visible nodes is the best way to redraw huge outlines because drawing actually visible nodes takes approximately constant time. Alas, drawing actually visible nodes is surprisingly tricky. It has low priority for me.

New summary

Pyzo's code is unlikely to help Leo. Design, not code, is where real progress lies.

Infelicities in Leo's code should be fixed carefully and incrementally, with full notice of any possible problems.

Edward

[vitalije]

Dear Edward,

I find that it was very difficult to discuss code improvement ideas with you. The most noticeable road-block for such discussions was your habit of not reading and therefore not understanding the idea. Of course it may be just my inability to express the idea clearly enough. After all I am not a native English speaker. In most cases our discussions were ended with you claiming that everything is sound and good enough in Leo's code. In your recent ENB post about Leo's outline redraw code you wrote again some claims that I truly  believe are wrong. For as long as you keep saying such claims to yourself there won't be any serious improvement possible. If you really want for us to have a meaningful discussion you have to be open to hear some critiques and to take necessary time to fully understand what the critique is about.

Recently someone on this list wrote some critique using poorly choice of words like "bad as usual" and similar. Your reaction was to almost ban him and there wasn't any further discussion. I can understand that such critiques are painful but you have to separate yourself from Leo's code and not take those critiques as if they were saying that you are bad. I am certain that nobody on this forum would feel for you any less than pure admiration for everything you do, for the way you write and share your thoughts with us and for all the hard work that you have done so far creating Leo. But in spite of our collective admiration for you as a person, there are some bad smells in Leo's code. OTOH when you start to defend Leo's code you don't hesitate to use words like "immature" or "lack of experience" regarding your opponents which is not very helpful either.

You were recently looking for a juicy problem to solve. I think there are quite a few juicy refactorings that should be done to improve Leo's code. However it seems that you are not particularly fond of refactoring Leo's code.

Our communication was sometimes enjoyable and successful like when we recently discussed deletePositionsInList method. To me the code looked obviously correct. I couldn't write any clearer explanation. The curse of knowledge left me helpless. Then you took your time and you dived into the problem and proved yourself that the code is correct. I would like if you do this for any code improvement idea we discuss. Take some time and dive into the problem before rejecting the idea or critique.

Now let's get back to your post about outline redraw code. You wrote:

It's all too easy to lose data when switching from one node to another.

and I would agree with this claim. It is too easy to lose data. However I believe you meant to say that this is the fact that can't be changed. To me this is true at the moment in Leo's present state, but it is not something we can't do anything about. Rather to me this is the code smell. This is something that needs to be fixed. In the past I have written several implementations of Leo's outline in different languages and GUI frameworks. In all cases I made very simple and clean way to select nodes without any data loss. You could say that those were just experiments and that this doesn't necessarily apply to Leo. But I am certain that Leo's tree select can be greatly simplified and improved providing that we make all necessary refactorings. I almost hear you saying that the selection code is complex because it has to be and there is nothing we can/should do about it. But I truly believe this argument isn't true. You just need to dive into the problem and take some time to think it over and you'll see that it really doesn't have to be that complex.

One of the biggest sources of complexity in this area is Leo's GUI wrapper/widget machinery. Over the years you have convinced yourself that this is the good idea. It separates Leo's core from the GUI and allows different GUI's to be attached to Leo's core. But that just isn't so. The benefits of this idea are not so big and yet the complexity it introduces is considerable. The separation is not perfect either. There are some leaks from GUI into the core. And it introduces one whole extra layer that makes reading and understanding code very hard. If Leo was written in Java or similar language this architecture would be necessary but in Python it doesn't provide anything substantial. The wrapper classes are like Java interfaces or Scala/Rust traits. However in Python those are not necessary at all. The main benefit of this classes is for writing a new GUI, but how often does it happen? Is it worth the effort? I really doubt it. Suppose we want to write a new GUI for Leo. The way you would do it I believe is to copy all wrapper classes in a new module and then start to implement methods that must be implemented. But you could also take qt classes and copy them into the new module and then replace the implementation of methods. Delete old implementation and write new. The only difference would be that in the second case you have to delete the qt implementation and in the first you have to replace self.oops() with the real implementation. What is the benefit then of having layer of wrapper classes? Is it the nice list of methods that must be implemented and methods that may be implemented? Well this list might be kept in the comments and copied from one GUI to another the same way it is now copied from wrapper classes.

You may say that this architecture allows a string GUI that is used for testing, but this is another story. Many of the unit tests in their present implementation are useless because they are not testing the production code but some other code that is never used. Wherever the `if g.unitTesting` is used the code that is being tested and the code that is being executed in normal usage are different. To see for your self how little of production code is being tested I suggest you to delete some methods from qt classes or even to replace some objects with totally different classes and then execute unit tests. For many of changes you can make this way the result would be all unit tests passed. One of the promises unit test make is that overall code quality improves. The more testable code usually means more flexible and better code. The more pure functions in the code the more testable code it becomes. That is why functions should be preferred over classes which is not in Leo's code at the moment. I am not saying that we should use no classes at all. Some classes are necessary but many of Leo's classes are not necessary and they just make testing code harder than it needs to be.

The interface between Leo's code is minimal.

This is another claim that I believe to be false. Before some recent changes I have made in this area, the outline drawing code was surprisingly spending more than 40% time in os.path.norm_path. Those are removed now but they are signs of deeper interconnection between the core and drawing code than you claimed to be. Drawing outline still has some calls to c.setCurrentPosition which really shouldn't be there If the principle of doing only one thing is followed. The presence of these calls in the drawing code to me is obviously wrong. I might not be able to explain it clearly enough. But I am sure if you dive into the problem with open mind you'll see that there can't be any logical explanation why these calls would be necessary inside the drawing code. From the present situation it is not clear which object is responsible for selecting and which is responsible for drawing. If the request for drawing is coming from commander then the commander should be responsible to call c.setCurrentPosition prior to calling tree.redraw() and tree.redraw should not call c.setCurrentPosition in any logical scenario. You might say that calling c.setCurrentPosition is benign but event if it is so, it adds more unnecessary complexity and prevents some other potential code simplifications. The c.hoistStack and c.chapterController are also deeply interconnected with the tree drawing and tree selecting. Of course they had to be, but perhaps c.hoistStack should not be in the core. Its purpose is totally gui related and it would be much better if this field was in the tree and not in the core commander. The core should not be concerned with how the tree is displayed but at present it is and it makes another bunch of interconnections between core and the view classes. All those unnecessary interconnections add to the complexity of the code. So the code is not as simple as possible but much more complex. It is easy to just dismiss all this facts and claim that complexity is unavoidable but that won't be true.

I could add many more items on the list of necessary refactorings but it would be useless unless you accept the fact that the code is not rock solid as you claim it is and there is in fact a great need to revise the code. Fixing bugs and adding new features almost always leads to sub-optimal code and refactoring is necessary from time to time to keep code manageable. However you wrote:

The present code is reasonable and rock solid. There is no great need to revise it, and I have no plans to do so.

And what can I or anyone else say to that? For such large scale refactorings you are the only one who can approve them. I am willing to do the necessary work, but I am not prepared to spend hours and weeks on the refactorings if you are not willing to accept them in the end. We have to at least agree that some refactoring is necessary.

There was a discussion on rewriting from scratch or using some other strategy for refactoring code. I have read about the mikado method as the good way to tackle refactoring of large projects. Basically it suggests to change any part of code and see what parts of the application are broken. Then revert the code in the initial state and make change somewhere else and record what is broken by this change. Using this technique one can learn quickly about hidden interconnections between modules. With this knowledge acquired it is usually possible to make some refactoring with greater confidence. When I read about this method I couldn't help but see the Leo as the perfect example of large project which needs to be refactored.

In the past I have made several attempts to improve drawing code. Most recently I worked on the tree-refresh branch which avoids deleting tree items and uses a fast diffing algorithm to find which items should be changed. It gives some improvements but not enough to be proposed as the way to go. However I am sure that in combination with some other refactorings like moving hoisting logic and chapter logic from core to the view and cleaning the drawing code from the calls to tree.select and c.setCurrentPosition this approach can be very good. Actually both ideas using diff algorithm to change the tree and not to recreate it from the scratch and the idea of drawing only handful of nodes that user can actually see can't give enough performance gain unless these other refactorings are done. Once they are done the drawing won't be a problem any more. Both of these ideas can't really shine until these refactorings are done.

There are many more possible improvements that depend on some of these large scale refactorings that can't be done without your approval. For example I've been working on the binary python extension written in rust and one written in cython. Both of them are very promising. I have tested them on both Linux and Windows. I can't test it on Mac because I don't have a Mac, but according to the cython and rust documentation there should be no problem compiling this extensions on Mac and they should work on Mac too. Using the extension written in rust I managed to load LeoPyRef.leo along with all at-file nodes in about 40-80ms. The tree has more than 8000 nodes. Traversing this tree using the binary extension takes less than 4ms. I haven't written yet the reading at-auto files, but this should not be too complicated either. I am quite confident that using this module for reading the whole tree and building vnodes from it to be used directly in the Leo as it is now could replace leoAtFile and leoFileCommands modules or at least functions from these modules could use binary extension if available to delegate their work to the binary extension and loading large outlines like LeoPyRef.leo would take about 200ms. The drawing code that would rely on the binary extension if available could make a great difference especially in the idea of using diff algorithm to just change the tree instead of building it from scratch every time c.redraw is called.

Vitalije

[Thomas Passin]

On Friday, May 1, 2020 at 11:09:10 AM UTC-4, vitalije wrote:

Dear Edward,

I find that it was very difficult to discuss code improvement ideas with you. The most noticeable road-block for such discussions was your habit of not reading and therefore not understanding the idea. Of course it may be just my inability to express the idea clearly enough. After all I am not a native English speaker. In most cases our discussions were ended with you claiming that everything is sound and good enough in Leo's code. In your recent ENB post about Leo's outline redraw code you wrote again some claims that I truly  believe are wrong. For as long as you keep saying such claims to yourself there won't be any serious improvement possible. If you really want for us to have a meaningful discussion you have to be open to hear some critiques and to take necessary time to fully understand what the critique is about.

[long snip]

I cannot give any practical suggestions as to changing or refactoring Leo's GUI code since I have not studied it.  But perhaps I can say something useful here anyway.

First of all, I haven't yet seen a compelling reason why a big effort in this area would be worthwhile.  Why, for example, would I care if the load time for LeoPyRef dropped to 80ms?  On my machine, the outline loads in about two seconds.  I'm happy with that - I don't see how a faster load time would matter.  Yes, I have a fast computer, but even say 10 seconds wouldn't bother me too much, since I don't have to load it often.  I have not been hindered by delays in navigating the outline, large though it is.

Yes, easier maintenance and reduction of technical debt would be good, but Leo works pretty well as it is.

I'm not saying there isn't a good case to be made, just that it's not apparent to me yet.

Second, because reworking large chunks of Leo's internals would be such a large job,  I suggest seeing if there is a smaller piece of it that would make sense to focus on.  For example, Edward had written several times about how clones make the outline display much harder than, say, for pyzo's equivalent.  Try concentrating just on that point for a while.  It's not so obvious to me, but I don't know anything about the code.  To my naive brain, I would think that one would collect the nodes of interest in a temporary list or tree, and then walk that tree to create the display.  The clone or otherwise status of nodes wouldn't play a part.  The list would only contain object references, not copies of actual nodes.

Yes, it's hard to know if a node would be off the bottom of the display, but we know how much vertical space a node requires to display, and so we can stop displaying when we have done than number (or more by some margin).  This is analogous to clipping lines when they would draw off screen.  Yes, It may not be so easy, but once the code has been set up, it just quietly does its job.

Well, maybe this is exactly how Leo already does it, I don't know.  My point is not about these details, but rather that maybe looking at *why* the clones make the drawing task hard - or really, if a different approach could be rather simpler - would be a profitable thing to do.

Third, when I read suggestions about changing Leo's GUI internals, I get concerned that some current plugins or other code won't work anymore.  Before any proposals for large changes were accepted, I would like to know that this issue had been looked at.  For example, I believe that the forward/backward arrows are provided by a plugin.  I would hate to lose them for a few years while Leo's GUI was being redone.

So my input here, for what it is worth, is that in principle I think that a more complete separation of GUI from non-GUI, and GUI from QT, would be desirable;  I would like to see a stronger look at the practical benefits, an effort to find a more limited area to concentrate on to serve as a proof of principle or at least to make the tangible benefits more evident, and a more system-oriented approach to make sure that we don't inadvertently lose important functionality.

[vitalije]

On my machine, the outline loads in about two seconds.  I'm happy with that - I don't see how a faster load time would matter.

Well you may be right about this. Maybe it isn't the speed that matters so much.

But when I think about why would I wish to contribute to the open source project. The reason for me is because I love writing code. I love elegant code, I love the "Aha" moments when I rewrite some code so that it becomes shorter, more readable, more elegant and quite often it becomes more performant at the same time. I hate when I see that some code forces CPU to do the unnecessary work. You might say who cares? But I do care. I like to see the code that works as little as necessary. As a Leo developer I wish I could feel good about Leo's code base. I don't like to feel even a slightest shame if someone looks at the deepest layers of Leo and sees there something poorly designed or coded. It really doesn't matter to me who wrote the code. If it is not best possible I feel guilty. I am sad when I see the code smells and can't do anything about it. Maybe if I were writing code for money I wouldn't be able to look at large refactorings without calculating the costs of it. However, since I don't code for the money it makes no difference to me whether it is the large refactoring or adding a new feature. The only thing that matters to me is whether the code would become more readable, more understandable, more manageable or not. Of course refactoring means improving code while keeping the same functionality. If I am not improving the code then I really don't know why would I wish to contribute anything at all? If the performing a large scale refactoring to make code better is boring and tiresome activity then why bothering with open source project in the first place? It is the final result (better code) that matters to me even if no user should ever notice any improvement at all. But more often than not when code gets better, program also runs faster and users are happier.

After some code improvements it might happen that a new feature emerges that couldn't possible be found before code cleaning. The bad shaped code prevents new ideas. We can't be sure which great features are we missing because of the code smells that are not addressed properly.

Vitalije

[vitalije]

As an example of the elegant code you can look at the returns package. Quite often in Leo None is used either as a return value from a function or as an argument to the function. Just look at the examples in the returns documentaion how using some other way to represent missing value simplifies and beautifies the code.

if user is not None:
     balance = user.get_balance()
     if balance is not None:
         balance_credit = balance.credit_amount()
         if balance_credit is not None and balance_credit > 0:
             can_buy_stuff = True
else:
    can_buy_stuff = False

# or using returns ...
def get_balance(user):
    return user.get_balance()
def credit_amount(balance):
    return balance.credit_amount()
def is_positive(x):
    return x > 0

pipeline = pipe(
    get_balance,
    credit_amount,
    is_positive,
)
can_buy_stuff = Maybe.from_value(user).map(pipeline)

Look how beutifully and how easy it is to compose pure functions in the pipeline and then mapping it to the Maybe value. And how nested and unreadable code becomes when you have to do with possible None values. Leo has many places where this pattern can be applied.

How easy it would be to test this function. There is no place for bugs to hide.

Vitalije

[Thomas Passin]

On Friday, May 1, 2020 at 12:54:53 PM UTC-4, vitalije wrote:

On my machine, the outline loads in about two seconds.  I'm happy with that - I don't see how a faster load time would matter.

Well you may be right about this. Maybe it isn't the speed that matters so much.

But when I think about why would I wish to contribute to the open source project. The reason for me is because I love writing code. I love elegant code, I love the "Aha" moments when I rewrite some code so that it becomes shorter, more readable, more elegant and quite often it becomes more performant at the same time.

I feel the same pull towards elegant code. I'm only suggesting that it will be hard to get buy-in from others to do a major project when it won't seem to affect the way Leo functions for them, and may for some (especially Edward) cause a great deal of extra work and entail some risk.  So maybe there is a smaller, well-encapsulated, area in which you can start off.  It can still be within the GUI code.

[Edward K. Ream]

On Fri, May 1, 2020 at 10:09 AM vitalije <vita...@gmail.com> wrote:

Dear Edward,

I find that it was very difficult to discuss code improvement ideas with you. The most noticeable road-block for such discussions was your habit of not reading and therefore not understanding the idea.

I'm not sure what idea you are talking about. I often don't understand complex ideas, which does not mean I want to reject them out of hand.

Recently someone on this list wrote some critique using poorly choice of words like "bad as usual" and similar. Your reaction was to almost ban him and there wasn't any further discussion.

I never said I would ban anyone. I simply said I stopped reading the offensive post.

You were recently looking for a juicy problem to solve. I think there are quite a few juicy refactorings that should be done to improve Leo's code. However it seems that you are not particularly fond of refactoring Leo's code.

Refactoring is difficult and dangerous, especially when the code is not covered completely by unit tests. Imo, there should be a strong reason for refactoring. That said, I think refactoring is the only way to improve or simplify code.

Our communication was sometimes enjoyable and successful like when we recently discussed deletePositionsInList method. To me the code looked obviously correct. I couldn't write any clearer explanation. The curse of knowledge left me helpless. Then you took your time and you dived into the problem and proved yourself that the code is correct.

Yes, that was a good collaboration.

I would like if you do this for any code improvement idea we discuss. Take some time and dive into the problem before rejecting the idea or critique.

My first thought is usually whether the idea seems important enough to devote the extra time. I don't recall ever rejecting an idea without reading it.

Now let's get back to your post about outline redraw code. You wrote:

It's all too easy to lose data when switching from one node to another.

and I would agree with this claim. It is too easy to lose data. However I believe you meant to say that this is the fact that can't be changed. To me this is true at the moment in Leo's present state, but it is not something we can't do anything about.

I'm open to suggestions in this area. This is a new discussion, one that we've never had before.

Rather to me this is the code smell...I am certain that Leo's tree select can be greatly simplified and improved providing that we make all necessary refactorings.

That would be great!

I almost hear you saying that the selection code is complex because it has to be and there is nothing we can/should do about it.

Please do not put words in my mouth.

One of the biggest sources of complexity in this area is Leo's GUI wrapper/widget machinery. Over the years you have convinced yourself that this is the good idea.

How else is Leo's core to remain unchanged while supporting multiple guis?

You may say that this architecture allows a string GUI that is used for testing, but this is another story. Many of the unit tests in their present implementation are useless because they are not testing the production code but some other code that is never used.

I disagree. In any case, this is yet another discussion.

>> The interface between Leo's code is minimal.

> This is another claim that I believe to be false. Before some recent changes I have made in this area, the outline drawing code was surprisingly spending more than 40% time in os.path.norm_path. Those are removed now but they are signs of deeper interconnection between the core and drawing code than you claimed to be.

The interface to which I was referring are the calls to c.redraw(). That is pretty close to the simplest api that could possibly work.

The calls to os.path.normpath look like a performance bug. This bug does not seem to affect Leo's class design in any way. I have not looked at the code in question. I am willing to trust your fix.

> Drawing outline still has some calls to c.setCurrentPosition which really shouldn't be there If the principle of doing only one thing is followed. The presence of these calls in the drawing code to me is obviously wrong.

Yet another topic for discussion. I don't remember anything about those calls or why they are there.

> ...I am sure if you dive into the problem with open mind you'll see that there can't be any logical explanation why these calls would be necessary inside the drawing code.

It's not a question of me having an open mind. This code is completely off my radar, and it's not something I care anything about at the present time. I emphasize those last words because I have no investment in the old code. If you have ideas for better redrawing code, I would be willing to listen to specific ideas, or better, to look at actual code.

> I could add many more items on the list of necessary refactorings but it would be useless unless you accept the fact that the code is not rock solid as you claim it is and there is in fact a great need to revise the code.

Perhaps we disagree about what the term "rock solid" means. What I mean is that in the past decade or so there have been no credible reports of people having lost data as the result of bugs in Leo's Qt code. This is a statement of reliability, not a statement of code simplicity or any other code metric.

> Fixing bugs and adding new features almost always leads to sub-optimal code and refactoring is necessary from time to time to keep code manageable. However you wrote:

The present code is reasonable and rock solid. There is no great need to revise it, and I have no plans to do so.

I'll summarize my attitude as follows. I am not aware of any reliability issues with the present Qt drawing code. I am open to specific suggestions for improving the code, but no change to the Qt code must ever lose data.

> For such large scale refactorings you are the only one who can approve them. I am willing to do the necessary work, but I am not prepared to spend hours and weeks on the refactorings if you are not willing to accept them in the end. We have to at least agree that some refactoring is necessary.

I can not comment about ideas that haven't been presented to me, nor about code that does not exist. General statements about the Qt code base won't convince me. I need something I can understand.

> There was a discussion on rewriting from scratch or using some other strategy for refactoring code. I have read about the mikado method as the good way to tackle refactoring of large projects. Basically it suggests to change any part of code and see what parts of the application are broken. Then revert the code in the initial state and make change somewhere else and record what is broken by this change. Using this technique one can learn quickly about hidden interconnections between modules.

An interesting idea.

> With this knowledge acquired it is usually possible to make some refactoring with greater confidence. When I read about this method I couldn't help but see the Leo as the perfect example of large project which needs to be refactored.

This kind of general statement is neither illuminating nor convincing. We are talking about Leo's data integrity here.

> In the past I have made several attempts to improve drawing code. Most recently I worked on the tree-refresh branch which avoids deleting tree items and uses a fast diffing algorithm to find which items should be changed. It gives some improvements but not enough to be proposed as the way to go.

Yes, I am convinced that you understand the general scope of the problems.

> However I am sure that in combination with some other refactorings like moving hoisting logic and chapter logic from core to the view and cleaning the drawing code from the calls to tree.select and c.setCurrentPosition this approach can be very good. Actually both ideas using diff algorithm to change the tree and not to recreate it from the scratch and the idea of drawing only handful of nodes that user can actually see can't give enough performance gain unless these other refactorings are done. Once they are done the drawing won't be a problem any more. Both of these ideas can't really shine until these refactorings are done.

I have said several times that I have no objection to qualified devs such as yourself attempting to improve and simplify Leo's code. However, I can not express an opinion about vague strategies or unseen code.

> There are many more possible improvements that depend on some of these large scale refactorings that can't be done without your approval.

What, exactly, do you want me to do? I have already given you approval to refactor any part of Leo's code on an experimental basis. Your fast read code was a spectacular success. Your proposal to redefine positions was a spectacular failure. Your prototype Tk gui code was way way better than my old Tk gui code.

There is no way for me to know what your Qt gui code will be like until I see it. Even then, the only way to know whether the new code will preserve data will be to use it for a few months.

> For example I've been working on the binary python extension written in rust and one written in cython. Both of them are very promising.

Yet another topic, with another set of considerations, the most important of which, to me, is the question of how to distribute binary code. Matt may have something to say about this.

Summary

Please do not generalize about my willingness to consider new ideas.

I frequently misunderstand ideas. Sometimes I am willing to do extra work on my own to understand proposals. Whether I am willing to do extra work depends on my guess about the importance of each proposal. That guess might be wrong. I know that. Don't give up.

To help me out:

- Do not assume I understand the implications of all of Leo's code. I don't.

- Make your proposals as clear, concrete and intuitive as possible.

  It often take me sustained, protracted work to understand new ideas.

- Do not lump proposals together! Give me a chance to understand each separately.

Edward

[Thomas Passin]

I almost hate to get into this, because it goes to show that everyone has a different notion of elegance and readability.  But I'm going to give you my reaction to your example here.  I only do this to highlight how it won't be so easy to replace the GUI code with something that many other people will want to deal with.

Your last line

can_buy_stuff = Maybe.from_value(user).map(pipeline)




is almost opaque to me.  map tends to be hard to understand - that's why Python has mostly replaced it with list comprehensions - and I don't know what Maybe or from_value are supposed to be.  Also, the names don't work together consistently.  get_balance is clearly a verb, is_positive is a condition or test, and credit_balance is a noun. IMO, they don't belong together in a pipeline as if they were the same kind of thing. 

So I really don't like the way you have constructed things here.  In addition is not as error free as you imagine because the same edge cases still exist in the three functions.  So you are actually making some errors harder to detect.   I'd rather see a fluent style, or even a simplification of the original, maybe something like this:

can_buy_stuff = False
if user:
   # get_balance() always returns a balance object, creating one with 0 credit if necessary
   credit = user.get_balance().credit_amount()
   can_buy_stuff = credit > 0

Or maybe something like this, which appeals more to me and seems more clear and self documenting than your pipeline:

# user must always have a balance, and balance must always have a credit

can_buy_stuff = user and user.balance.credit > 0

The guarantee of having balance and credit could be established at object creation.

Anyway, I'm sure there is lots of scope for more elegance in Leo's code, but there is more than one way.  It will take quite some thought.  I know that you have put a lot into your various experiments, I'm not trying to minimize that.  But maybe some new capability could be better implemented by applying your lessons, instead of a major re-write of the core all at once.

[Edward K. Ream]

On Fri, May 1, 2020 at 11:02 AM Thomas Passin <tbp1...@gmail.com> wrote:

> I cannot give any practical suggestions as to changing or refactoring Leo's GUI code since I have not studied it.  But perhaps I can say something useful here anyway.

> First of all, I haven't yet seen a compelling reason why a big effort in this area would be worthwhile.

[snip]

> Second, because reworking large chunks of Leo's internals would be such a large job,  I suggest seeing if there is a smaller piece of it that would make sense to focus on.

[snip]

> Third, when I read suggestions about changing Leo's GUI internals, I get concerned that some current plugins or other code won't work anymore.

[snip]

Thanks for these comments. I think they are right on target. I'll say more soon.

Edward

[Edward K. Ream]

On Fri, May 1, 2020 at 11:54 AM vitalije <vita...@gmail.com> wrote:

> when I think about why would I wish to contribute to the open source project. The reason for me is because I love writing code.

Vitalije, my heart melted when I read this :-) I share your motivations. Oh sure, I can invent "larger" reasons for working on Leo, but for me the love of writing code is primal.

> the bad shaped code prevents new ideas.

Bad code can be an invitation to new ideas, and better code :-)

Vitalije, I've been thinking of our discussions overnight. My wish for you is that you feel free to explore all your ideas:

1. You do not need my permission to begin. Only you can understand your ideas well enough to assess and estimate the risks involved.

2. You do not need my permission to experiment with code in separate branches. When you are ready, the entire Leo community will evaluate the results, using the usual metrics.

The best plans and code have obvious merit. For example, the new read code was clearly simpler and faster than the old. Accepting it was a no brainer. My big mistake was accepting it too quickly :-)

Risk

What to do about the risk inherent in major undertakings? Here are several suggestions:

1. Don't wait for me to assess risk. Trust yourself.

2. Consider writing Engineering Notebook posts on leo-editor:

- Writing is the best way of clarifying choices, tradeoffs, and risks.

- Leo's community will be interested in your public ENB posts:

  comments, suggestions and questions are your friends.

- Public writing activates the unconscious. Listen for whispered (or shouted) doubts.

- Writing is a necessary check on the primal urge to write code :-)

3. Focus on the smallest projects that make sense.

Summary

Vitalije, my wish for you is that you explore all your ideas. I'll support you in any way I can.

You are in charge of your dreams, plans and actions. You alone are responsible for assessing and mitigating risks.

As one of Leo's most important devs, it is entirely appropriate for you to write Engineering Notebook posts on Leo's forum. Many people will be interested.

Focus on small projects. Small is a relative term: your read code was a beautiful sweet spot. Only you can determine the appropriate scale of your projects.

Edward

[vitalije]

Vitalije, my wish for you is that you explore all your ideas. I'll support you in any way I can.

You are in charge of your dreams, plans and actions. You alone are responsible for assessing and mitigating risks.

As one of Leo's most important devs, it is entirely appropriate for you to write Engineering Notebook posts on Leo's forum. Many people will be interested.

Focus on small projects. Small is a relative term: your read code was a beautiful sweet spot. Only you can determine the appropriate scale of your projects.

Edward

Thanks for these kind words. My heart melted too when I read them. I can't write more at the moment. I've started this morning to write a new experiment - prototype an Qt application which has just three widgets: body, tree and toolbar. It uses VNodes but not positions. Tree is drawn only once and every tree item has attached v node. The selection works correctly with no data loss. Editing body text is correctly preserved when switching from one node to the other. Editing headlines works too. I want to add methods for changing the outline moving nodes up,down,left and right,promote and demote, inserting new nodes, deleting nodes.

The idea is that each of these command should change links simultaneously both between the tree items and v-nodes. That way tree items will always be in sync with v-nodes. But when the re-synchronization is required I plan to use the diff algorithm that I have already implemented in the tree-refresh branch.

I will be writing in more detail in ENB once I finish this prototype so that we can discuss the actual code. It seems that the code will be easier to understand than my explanations.

In two or three days I think I will have something to show.

Vitalije

[Matt Wilkie]

It is inspiring to see people with deep skills and experience willing to engage in hard conversations and explore where to go next, to not drop out of the game or flame up when confrontation arises.

-matt

[jkn]

On Tuesday, May 5, 2020 at 12:20:31 AM UTC+1, Matt Wilkie wrote:

It is inspiring to see people with deep skills and experience willing to engage in hard conversations and explore where to go next, to not drop out of the game or flame up when confrontation arises.

-matt

I fully agree - thanks for the good example ;-)