ENB: About PR #4637

[Edward K. Ream]

I am about to merge PR #4637. Thomas rightly points out that this PR is too big for comfort. I would go further: the Qt gui is too complex to understand. This Engineering Notebook post attempts to convince you that I do know what I'm doing. I'm not fully convinced :-)

I have spent days wrestling with the implications of even the slightest change this PR makes. For example, LeoQtGui.widget_name returns a best guess of what the widget's name is. This "guess" is used (with significant consequences!) in dozens of places. The PR changes this method in a subtle way. It now guarantees to return a non-empty string. Previously, it could return an empty string. Does this matter? I don't think so, but I wouldn't bet my life on it. Furthermore, a case could be made for eliminating the LeoQtGui.widget_name completely, thereby using the LeoGui base class method instead. But I think retaining the subclass method makes sense.

The PR contains a milestone: a real test of Qt event handling. Alas, this test is a shadow of what it would need to be to test the recent problems with handling events in the Completion pane. But this test is feeble. I spent all day yesterday trying to track down which event filters apply to which widgets. Without much real success!

While working on the unit test, I discovered and removed three or four more code horrors. Were these changes risky? You bet they are. But imo it would be intolerable to leave the horrific code in place.

I also discovered some bugs in __repr__ and tracing code. Fixing those changes should be safe enough.  In particular, I improved the tracing code in k.masterKeyHandler and its helpers. But there is absolutely no way I am ever going to change Leo's key-handling code in any significant way. Any functional change would break users' key bindings. 

Summary

The PR lists seven significant changes. Imo, these changes belong in a single PR. Any of these changes could have serious unintended consequences. If so, I'll track them down with git bisect.

The takeaway conclusions:

- Leo's devs must not try to change Leo's gui code unless absolutely necessary!

- Leo's devs must not ever change k.masterKeyHandler and its helpers.

But, big sigh, the PR does change ec.doPlainChar. And yes, you could say that doPlainChar is one of k.masterKeyHandler's helpers. I can live with that fact, for now.

Edward