Active settings outline: status report

[Edward K. Ream]

In another thread I said, "I'm going to do #852: (active settings outline) first.  I think I can do this in a few hours [sic] to a few days.  It's a frequently requested feature."

I spent about 15 hours on yesterday, from 2 am to 7pm, with just a few breaks.  And here I am again, at 2:30 am. This is a difficult, fascinating, project. It took all yesterday to come up to speed on Leo's configuration code.  The "settings" branch contains the work.

The following are some notes, feel free to ignore.  They might find their way into an info issue dealing with settings. I am writing this now to summarize yesterday's researches.

About the active-settings-outline command

The ActiveSettingsOutline class implements this command. At present, the code creates an new, real, outline illustrating the overall form of the final outline.  You can save this outline if you want.  It's name is x-active-settings.leo, where x is the name of the outline that executed the active-setting-outline command.  This was the easy part ;-)

The aso.start method, and its helpers, now appears to recreate the required initing of settings exactly as is done in Leo's startup code. Subtle bugs will abound if it doesn't. The code is deceptively simple. There are a frightening number of constraints on this code.

The aso.create_active_settings method shows the difficulties involved in traversing the various @settings trees while simultaneously seeing where the settings come from. Despite the present complications, I am pretty sure the code can be simplified.

About the configuration code

The c.config.get* methods are the one and only interface between this code and the rest of Leo.  This interface hides the truly frightening complexity of the underlying code. 

The LoadManager class, in leoApp.py, does the heavy lifting involved with reading settings files, leoSettings.leo, myLeoSettings.leo, the theme file, specified by the @string theme-name setting (usually in myLeoSettings.leo) and the local files, the actual outline files.  Leo can load leoSettings.leo, myLeoSettings.leo and any theme file, so that has to work too. That is, local files can also be settings files or theme files.

I'll abbreviate the singleton g.app.loadManager instance by LM, which I also use to denote the LoadManager class.

Leo already caches settings.  There are two global settings dicts, LM.globalSettingsDict and LM.globalBindingsDict.  And there is a corresponding local settings dict, c.config.settingsDict. Not sure whether there is a local bindings dict :-)

The global dicts represent the settings in both leoSettings.leo and myLeoSettings.leo. Not sure whether I would do things the same way if I were starting from scratch.  Important: the local settings dict contains all the settings available to the outline.  That is, c.config.settingsDict is the union of all settings from settings files, the theme file and the local file.  This union gives first priority to the settings in the local file, then the settings in the theme file, and last, the settings from the settings files (the global settings dict).

This union is an important optimization. The fundamental c.config.get method needs only access one dict, namely c.config.settingsDict.  In other words, the get method knows nothing about search order.  That has already been resolved.

Each value in the c.config.settingsDict is a g.GeneralSetting (GS) instance.  GS objects contain a path ivar, which is the full path of the .leo file that is responsible for the setting.  This path allows gcm.config_iter to determine where the setting came from.  As I write this, I see that active settings outline will use gcm.config_iter as a model for assigning settings to files.

That's all I want to say about the configuration code.  More details would be useless.  Most people, including myself, have no chance of remembering anything more.

About changing the configuration code

I plan to make zero substantive changes to the configuration code while working on #852.  The only changes I have made are to docstrings.  I did make one "hidden" change to readGlobalSettingsFiles.  It now sets LM ivars needed for the ActiveSettingsOutline class. Even this tiny change was worrisome enough that I created a new git branch for the work.

It's extremely difficult to know what the code does, in exact detail.  A trace in LM.openSettingsFile shows that reading settings files happens in two stages.  LM.doPrePluginsInit does the first stage, LM.doPostPluginsInit does the second stage.  The ActiveSettingsOutline code must use exactly this load order in order to faithfully recreate settings.  Btw, the --theme=name command-line argument changes the move order.

Given the complexities of the code, I simply do not understand how anyone can be confident that proposed changes would work exactly as before.  After 15 hours of intense study, I don't understand the code well enough to have any confidence in any real changes to the code.

Global settings and problems

The node "c.config.Getters: redirect to g.app.config" contains problematic code.  It contains four methods: 

def getButtons(self):
    '''Return a list of tuples (x,y) for common @button nodes.'''
    return g.app.config.atCommonButtonsList # unusual.

def getCommands(self):
    '''Return the list of tuples (headline,script) for common @command nodes.'''
    return g.app.config.atCommonCommandsList # unusual.

def getEnabledPlugins(self):
    '''Return the body text of the @enabled-plugins node.'''
    return g.app.config.enabledPluginsString # unusual.

def getRecentFiles(self):
    '''Return the list of recently opened files.'''
    return g.app.config.getRecentFiles() # unusual

The first two probably don't cause great problems.  The second two are somewhat dubious.  I'll leave it at that for now. Anyone seriously considering changes to the config code should be aware of these hacks.

A possible source of theme misery

While studying LM.readGlobalSettingsFiles I saw the following code:

if 0:
    # Not necessary **provided** that theme .leo files
    # set @string theme-name to the name of the .leo file.
    g.app.theme_color = settings_d.get_string_setting('color-theme')
    g.app.theme_name = settings_d.get_string_setting('theme-name')

Whoa!  What's going on here?  There is almost zero chance I'll enable this code.  But I may well issue a warning (somewhere!) if a theme file does not contain an @string theme-name setting.  This would be oh so hard to find.  It's a dog that isn't barking.

Summary

Work on the active-settings-outline command is progressing well.  A day or two more should suffice. All work is being done in the settings branch. I shall make no substantive changes to Leo's configuration code.

The active-settings-outline command must recreate Leo's startup code exactly.  Subtle bugs will abound if it doesn't.  If the command does init settings correctly, gcm.config_iter shows how to find where settings come from. This is possible because settings contain the path of the file that define them.

All theme files should defined @string theme-name to be the name of the theme file itself.  If not, the theme file may not work properly. I plan to add a warning if this is not so.

Edward

[vitalije]

Given the complexities of the code, I simply do not understand how anyone can be confident that proposed changes would work exactly as before.  After 15 hours of intense study, I don't understand the code well enough to have any confidence in any real changes to the code.

If you, a rather clever, smart, experienced developer and also the author of this code,  after 15 hours of intense study do not fully understand the code, shouldn't that tell you something about the quality of the code? If you cant fully understand the code, what chances to understand it will have the rest of us.

Now what I really don't understand is your decision not to fix this code even after you yourself had learned that it is far from being readable and maintainable code. Why don't you make it simple and readable? I could help if you need any help.

I don't understand the code well enough to have any confidence in any real changes to the code.

That is precisely why the real changes are necessary. I would say I don't have any confidence that it works correctly at all in its current state. That should be well enough reason to refactor this code, no matter how much time and effort will it take. But I guess, you'll prefer to leave this code as it is.

I am tempted to put aside all my current tasks and write my own launchLeo.py which will monkey patch completely cofiguration code and just read settings from the database table. I guess it would greatly reduce number of code lines that has to be executed every time Leo opens new commander. 

Vitalije

[Edward K. Ream]

On Sun, Sep 1, 2019 at 5:13 AM vitalije <vita...@gmail.com> wrote:

If you, a rather clever, smart, experienced developer and also the author of this code, after 15 hours of intense study do not fully understand the code, shouldn't that tell you something about the quality of the code? If you cant fully understand the code, what chances to understand it will have the rest of us.

I wondered whether you might say something like this.

Now what I really don't understand is your decision not to fix this code even after you yourself had learned that it is far from being readable and maintainable code. Why don't you make it simple and readable?

I would be happy to make the code simpler if I knew how to do this without damaging Leo.  I do not want to make Leo's users change their settings files, and I do not want to inflict subtle changes on now Leo handles settings. This seems virtually impossible.

 

I could help if you need any help.

Hmm.  First, you, Vitalije, have got to convince me that you understand the issues involved.

I don't understand the code well enough to have any confidence in any real changes to the code.

That is precisely why the real changes are necessary. I would say I don't have any confidence that it works correctly at all in its current state. That should be well enough reason to refactor this code, no matter how much time and effort will it take. But I guess, you'll prefer to leave this code as it is.

This seems to be the root of our disagreement. This would be no ordinary refactoring.

I am tempted to put aside all my current tasks and write my own launchLeo.py which will monkey patch completely configuration code and just read settings from the database table. I guess it would greatly reduce number of code lines that has to be executed every time Leo opens new commander. 

 

To gain my approval, you would have to convince me of the following:

Requirement 1. That users presently are suffering from configuration related bugs, that can not be fixed by small changes to the present code base.

Requirement 2. That you have a solid plan to avoid any proposed refactoring from introducing changes, of any kind, in how users perceive Leo's settings.

I would discourage you from this attempt, unless you have a clear plan for meeting requirement 2.

Summary

Your desire to mess with Leo's settings machinery strikes me as misguided and dangerous.  This is NOT primarily a code issue.  It's a management issue involving Leo's stability.

You have not demonstrated anywhere near the required understanding of Leo's configuration code. Insulting comments about the code's quality shows engineering immaturity. Yes, the code is difficult.  That in no way proves that it could be simplified safely.

If you do have a plan for meeting requirement 2, and can convince me of the plan's simplicity and soundness, then I shall naturally be interested. 

Edward

[Edward K. Ream]

On Sunday, September 1, 2019 at 3:53:51 AM UTC-5, Edward K. Ream wrote:

The aso.start method, and its helpers, now appears to recreate the required initing of settings exactly as is done in Leo's startup code. Subtle bugs will abound if it doesn't.

Oh joy.  There is a simple way around these difficulties.  The command's startup code will make deep copies of the dicts needed to show setting accurately.  The code that creates the summary outlines will use these copies, rather than the code created by the startup.

This is a slight of hand.  We definitely do want to do the full startup logic, so that, for example, abbreviations, key bindings, theme appearance, etc. etc. are as expected.  But we don't want to bet that there are no subtle differences between the logic in the local file and the logic in the active outline settings file.

This is a simple, sound plan to ensure that the active-setting-plan command will accurately show settings, even though it exists in a new commander.  In other words, it satisfies both requirements I have just challenged Vitalije with.

Edward

[vitalije]

Insulting comments about the code's quality shows engineering immaturity. Yes, the code is difficult.  That in no way proves that it could be simplified safely.

I am truly sorry if you read my comments as insulting. That was never my intent. Do you really think I would try to get your approval for anything while at the same time insulting you? That would be rather foolish from me.

Now when I reread what I have written the only remarks I have made about the code is asking you to recheck the quality of your code using your own recent experience with it. And in the next paragraph I wrote it is far from readability and maintainability which is how I would call any code that the author can't fully understand "after 15 hours of intense study". If you find this to be insulting I really don't know how we can ever hope to have a meaningful discussion about code.

Again I do apologize if my comments has hurt your feelings. From now on I will try not to comment anything about your code, unless you explicitly ask for my opinion. So, if you ever wish my comments, please ask.

Vitalije

[Edward K. Ream]

On Sunday, September 1, 2019 at 7:29:59 AM UTC-5, Edward K. Ream wrote:

I would be happy to make the code simpler if I knew how to do this without damaging Leo.

Aha!  I see a way forward.

 To gain my approval, you would have to convince me of the following:

Requirement 1. That users presently are suffering from configuration related bugs, that can not be fixed by small changes to the present code base.

Requirement 2. That you have a solid plan to avoid any proposed refactoring from introducing changes, of any kind, in how users perceive Leo's settings.

Requirement 1 suggests a completely new idea.  This idea suggests the way to meet requirement 2.

I agree with Vitalije that we don't really know how sound Leo's existing configuration code is.

Lets go about testing that present code with something like a prototyping unit test. This won't be a real unit test.  It would likely take way too long for that.  But it will be crucial, no matter how long it will take.  The unit test might well write results, including intermediate data, to a test cache, which might well be an external file.

Like any good unit test, it will compute expected settings without using Leo's existing code.  It will then compare the expected settings with the actual settings.

This is a breakthrough, in two, no three ways:

1. The faux unit test should give us some confidence in the present code.

2. It allows anyone to experiment with new code safely.  Any new code must pass the test.

3. The unit test itself will be a test bed for computing settings.

The unit test won't be simple, but it is free to be as simple as possible.

Summary

A prototyping unit test will relatively simple and intuitive. To be a proper unit test, it should work from first principles, ignoring Leo's actual code. The unit test will form an experimental test bed for alternative ways of delivering settings.

Such a unit test will show:

- Requirement 1:where Leo's present machinery presently fails.

- Requirement 2: where any proposed refactoring fails.

This is an important day in Leo's history.  It breaks the deadlock between Vitalije and me.

Edward

[Edward K. Ream]

On Sunday, September 1, 2019 at 8:20:31 AM UTC-5, vitalije wrote:

Insulting comments about the code's quality shows engineering immaturity. Yes, the code is difficult.  That in no way proves that it could be simplified safely.

I am truly sorry if you read my comments as insulting. That was never my intent. Do you really think I would try to get your approval for anything while at the same time insulting you? That would be rather foolish from me.

Apology accepted ;-)  The Aha I have just described would not have been possible without our difficult discussions.

Edward

[Brian Theado]

On Sun, Sep 1, 2019 at 6:13 AM vitalije <vita...@gmail.com> wrote:

I am tempted to put aside all my current tasks and write my own launchLeo.py which will monkey patch completely cofiguration code and just read settings from the database table. I guess it would greatly reduce number of code lines that has to be executed every time Leo opens new commander. 

Very slightly related to this, I was playing around and discovered leo can be started without reading any .leo files at all. Just delete config/leoSettings.leo and then run it like this:

HOME=$HOME/tmp/blank python launchLeo.py ~/tmp/new.leo

where ~/tmp/new.leo is a non-existing file (so no settings are are read from it) and $HOME/tmp/blank doesn't have any myLeoSettings.leo.

Surprisingly leo almost works when launching like this. There are no menus or keystrokes or right click menus, but mouse clicks work and you can mouse click onto the minibuffer and run commands. However outline modifications (i.e. insert-node) cause a core dump with this python stack dump:

Traceback (most recent call last):

  File "/home/btheado/src/leo-editor/leo/plugins/qt_frame.py", line 3369, in tab_callback

    c.findCommands.startSearch(event=None)

  File "/home/btheado/src/leo-editor/leo/core/leoFind.py", line 596, in startSearch

    self.openFindTab(event)

  File "/home/btheado/src/leo-editor/leo/core/leoFind.py", line 531, in openFindTab

    g.app.gui.openFindDialog(c)

  File "/home/btheado/src/leo-editor/leo/plugins/qt_gui.py", line 209, in openFindDialog

    d.setStyleSheet(c.active_stylesheet)

AttributeError: 'Commands' object has no attribute 'active_stylesheet'

I'm able to get this same crash by just clicking on the find tab's tab. It seems strange outline modification causes openFindTab to be called. 

While this is likely not a valid mode to be running leo, maybe it does point to some actual issue?

I'm surprised it causes core to be dumped.

Brian

 

[Edward K. Ream]

On Sun, Sep 1, 2019 at 8:32 AM Brian Theado <brian....@gmail.com> wrote:

> I...discovered leo can be started without reading any .leo files at all. Just delete config/leoSettings.leo and then run it like this:

    HOME=$HOME/tmp/blank python launchLeo.py ~/tmp/new.leo

Clever and creative. The prototyping unit test for config settings is free to use similar tricks.  This is the dawn of new era in Leo's horrible settings code.

Edward

[Edward K. Ream]

On Sunday, September 1, 2019 at 8:27:09 AM UTC-5, Edward K. Ream wrote:

This is a breakthrough, in two, no three ways:

And there is a fourth breakthrough.  The faux unit test allows for controlled, limited changes to how Leo's long-suffering users actually perceive settings.

For example, as described earlier, there is a single, global setting for enabled plugins. This naturally interferes with unloading plugins. I remain dubious that most plugins can be disabled without fully reloading Leo. But now we have a way to experiment with alternatives.

Vitalije, should you choose to accept it, I would like to name you as lead engineer for the prototyping unit test.  This will acquaint you with all the difficulties, and all the possibilities :-)

For now, I would like to focus on the active-settings-outline command, using the code as it is.

Edward

[Edward K. Ream]

On Sunday, September 1, 2019 at 8:48:15 AM UTC-5, Edward K. Ream wrote:

> Vitalije, should you choose to accept it, I would like to name you as lead engineer for the prototyping unit test.

I have just created #1314 for this.

Edward

[vitalije]

>
> Vitalije, should you choose to accept it, I would like to name you as lead
> engineer for the prototyping unit test.  This will acquaint you with all
> the difficulties, and all the possibilities :-)
>

I gladly accept the challenge. I am not sure what you mean by unit test,
but here is how I see it.

I will make a python module with the following functions:

• parse_settings_tree(vnode) - returns a generator of settings in the subtree of given vnode. Yields tuples (type, value)
• make_conf(filenames) - returns an object with the single method:
  
• get(name, kind) - replacement for c.config.get
  
• save_conf(filename)
  
• load_conf(filename)
  
• show_current_settings_tree(c, p) - this will rebuild settings outline with all values in effect as a first child of p, so that user can edit settings and root of this outline will have heading '@current-settings'
• update_current_settings_and_close(c) - finds node `@current-settings`, process them and updates values in c.config and perhaps stores them in ~/.leo/config.db

A unit test then can look something like this:

import mymodule
conf = mymodule.make_conf(['leoSettings.leo', 'myLeoSettings.leo',
'activeUnitTest.leo'])
d = c.config.settingsDict
for name, gs in d.items():
    assert conf.get(name, gs.kind) == gs.val

A couple of other tests should prove that save_conf/load_conf work as expected.

If this succeeds then Leo can use this module in the following way. Instead  of editing myLeoSettings.leo, user can execute 'show-current-settings-tree' which will dynamically show entire tree of settings where user can edit any
setting, and then execute 'update-current-settings-and-close' and this tree  will disappear, but changes will be stored for the next run, with the same effect as if myLeoSettings.leo was edited.

Show current settings tree should generate outline similar to leoSettings.leo with settings grouped by category.

That will allow us to direct user how to find any given settings. Every user will see the same shape of the settings outline. There could be a variation that will show only settings that user has changed comparing to Leo defaults. Or a variation where only settings with given search-string are shown filtering all other settings out.

Your comments please.
Vitalije

[Edward K. Ream]

On Sun, Sep 1, 2019 at 9:37 AM vitalije <vita...@gmail.com> wrote:

I gladly accept the challenge.

Thank you!

I am not sure what you mean by unit test, but here is how I see it.

In this context, a unit test is simply a script that verifies, in some unspecified way, that exiting code works.  It can also be applied to any proposed refactoring/simplification of the code.

To do both, the unit test should be self contained.  That is, it should not use existing code, but instead work from "first principles", that is, the highest-possible description of what is intended.  I think of such tests as a form of double-entry bookkeeping.

The script complements Leo's existing unit tests. In my mind, the script should be substantially more ambitious. It should test, as much as possible, all of Leo's settings, verifying that settings work as expected no matter where they are defined.  This kind of test might take minutes.

I will make a python module with the following functions:

• parse_settings_tree(vnode) - returns a generator of settings in the subtree of given vnode. Yields tuples (type, value)
• make_conf(filenames) - returns an object with the single method:
• get(name, kind) - replacement for c.config.get
• save_conf(filename)
• load_conf(filename)
• show_current_settings_tree(c, p) - this will rebuild settings outline with all values in effect as a first child of p, so that user can edit settings and root of this outline will have heading '@current-settings'
• update_current_settings_and_close(c) - finds node `@current-settings`, process them and updates values in c.config and perhaps stores them in ~/.leo/config.db

A unit test then can look something like this:

import mymodule
conf = mymodule.make_conf(['leoSettings.leo', 'myLeoSettings.leo', 'activeUnitTest.leo'])

d = c.config.settingsDict

for name, gs in d.items():
    assert conf.get(name, gs.kind) == gs.val

A couple of other tests should prove that save_conf/load_conf work as expected.

Very good.  I'll leave the details to you.

If this succeeds then Leo can use this module in the following way. Instead of editing myLeoSettings.leo, user can execute 'show-current-settings-tree' which will dynamically show entire tree of settings where user can edit any setting, and then execute 'update-current-settings-and-close' and this tree will disappear, but changes will be stored for the next run, with the same effect as if myLeoSettings.leo was edited. 

This is pretty much what I am working on now.  We can think about merging our respective code later.

Show current settings tree should generate outline similar to leoSettings.leo with settings grouped by category.

Right.  That's what active-settings-outline does.

Your comments please.

I'm glad we are now thinking in code.  This is much better than (necessarily!) vague discussions.

This new testing pattern can be applied to any proposed major change to Leo.  The application of this pattern will necessarily be creative, and will be unique to any big endeavor.  For the first time ever, I am confident that Leo will continue to evolve safely after I am gone.

Edward

[Edward K. Ream]

On Sunday, September 1, 2019 at 12:46:51 PM UTC-5, Edward K. Ream wrote:

Some second thoughts. We have drastically changed mind sets.  In my previous thinking, everything was dangerous.  In the context of the new testing script, everything is allowed.

The overall goal is to know where we have been and where we are going.  In this new world, there are no strict rules. The following aren't even suggestions.  They are just new possibilities that have appeared...

I am not sure what you mean by unit test, but here is how I see it.

In this context, a unit test is simply a script that verifies, in some unspecified way, that exiting code works.  It can also be applied to any proposed refactoring/simplification of the code.

Still true :-)

To do both, the unit test should be self contained.  That is, it should not use existing code, but instead work from "first principles", that is, the highest-possible description of what is intended.  I think of such tests as a form of double-entry bookkeeping.

Let me modify and extend this statement in several ways:

First principles are indeed an important guide. Leo's documentation might suggest new tests.

Having said that, testing scripts should be allowed to "peek" into Leo's internals, both to verify what the existing state of affairs is, and to re-verify what new code does.  The "peeks" can be different in each case.

We probably do want to start by using only Leo's api.  In this case, c.config.get* is a primary resource. However, we should feel free to add to Leo's api, to make testing easier.

I'll soon add an "official" method, say, c.config.getSource(aGeneralSetting).  aGeneralSetting will be a g.GeneralSetting, returned by c.config.get*.  c.config.getSource will return a string, one of ('leoSettings', 'myLeoSettings', 'theme', 'local', 'unknown', 'error').

The script complements Leo's existing unit tests. In my mind, the script should be substantially more ambitious. It should test, as much as possible, all of Leo's settings, verifying that settings work as expected no matter where they are defined.  This kind of test might take minutes.

Nah, it shouldn't take minutes.  Tests should just verify the basics:

1. They should verify the basic settings precedence:

- Settings in local files override all other settings.

- Settings in theme files override settings in myLeoSettings.leo and leoSettings.leo.

- Settings in myLeoSettings.leo override settings in leoSettings.leo.

2. Tests would also verify basic stuff like:

- That all of the basic settings nodes (@bool, @string, @int, etc) work as expected.

- That the global, special case nodes (@button, @command, @enabled-plugins) work as expected.

3. (Very important) that loading a new local file works as expected.  Ditto for theme files.

All these tests will likely be clearer and faster if they are applied to smallish versions of local files, theme files, myLeoSettings.leo and leoSettings.leo. For testing, we could even modify the search order so that testing files are found first.

Summary

We are free to do virtually anything in the new testing framework.  There are no strict rules.

The overall goal is to know where we are, and where any new code takes us.

A thorough testing framework may reveal edge cases in the existing code, that would be difficult, or foolish, to duplicate in proposed new code.  I'm not ruling out changing such edge cases, provided we do so consciously, and document what we have done.

Edward