ENB: Aha: use @c.command to split leoCommands.py

[Edward K. Ream]

This Aha is a direct result of our discussions in #581: Define all commands using a LeoCommand class?

The problem

The present Commands class (in leoCommands.py) is way too large. Besides providing essential services, the Commands class also defines commander commands. These are commander methods, like c.save, c.convertAllBlanks, etc., that define Leo's minibuffer commands. This was a design mistake, as I'll now explain.

Potentially any of these commands can be accessed in user scripts. For example, many of my own scripts call c.save(). As a result, all the command-related methods that now exist in the Commands class must continue to exist forever.  Removing any of these would break scripts of which we Leo devs have no knowledge.

Previously, there was no safe way of defining commander commands outside of the Commands class. This was the design mistake.

The Aha

The @c.command decorator, defined in leoCommands.py, will define a command, just like g.command. And (Aha!) the decorator will also inject the function (as a method) into the Commands class.  Voila!  This creates a safe and convenient way of defining commander commands outside the Commands class and outside of leoCommands.py.

For example, the c.save member will continue to exist even though the save function will reside in another file.

At present, commander commands reside in: class Commands-->c.Top-level commands. They are organized by menu. These commander commands will be moved out of the Commands class into corresponding files in the leo/commands folder: commanderFileCommands.py, commanderEditCommands.py, etc.

Summary

Once the @c.command decorator is proven to work we can:

- Migrate commander commands out of leoCommands.py safely and conveniently.

- Close #581. There will be no need to define commands using classes.

@c.command might also be able to handle complications such as c.universalCallback.  Or not.

Edward

[Edward K. Ream]

On Thursday, November 23, 2017 at 1:27:38 AM UTC-6, Edward K. Ream wrote:

The Aha

The @c.command decorator, defined in leoCommands.py, will define a command, just like g.command. And (Aha!) the decorator will also inject the function (as a method) into the Commands class.

It might be better to define new subcommanders to "hold" commander commands.  For example, the CommanderFileCommands class would define a save method which the the @c.command decorator would inject into the Commands class.

But what to do about existing assignments c = self in commander commands?

Could they be translated safely to c = self.c? It's worrisome...

So we have a dilemma.  One the one hand, it's always better to use classes to encapsulate related functions.  Otoh, it would be intolerable for text transformations to introduce subtle bugs in commander commands.  Existing unit tests don't cover all the cases. I'll have to think about this...

Edward

[Edward K. Ream]

On Thursday, November 23, 2017 at 1:40:25 AM UTC-6, Edward K. Ream wrote:

So we have a dilemma.  One the one hand, it's always better to use classes to encapsulate related functions.  Otoh, it would be intolerable for text transformations to introduce subtle bugs in commander commands.  Existing unit tests don't cover all the cases. I'll have to think about this...

Emacs never suffered from this dilemma because elisp had (then) no classes :-) All Emacs "macros" exist in a global namespace.

Rather than risk having to change the sources for commander commands, we could just use functions that "masquerade" as methods.  Like this, at the top-level of commanderFileCommands.py:

@g.commander_command('save')
def save(self, event=None, fileName=None):
    # Exactly the same code as before.

Notes:

1. @c.command won't work, because c doesn't exist at the top level.  And it probably wouldn't work within a class either.  So we will probably want to define the @g.commander_command decorator. It should be no big deal.

2. Pylint might complain about functions having a "self" arg.  All such complaints can be disabled at the start of the file.

3. I'm not sure what pyflakes will do.  This could be a show stopper for using functions. I'll check this immediately.

Summary

The dilemma isn't resolved.  It may, after all, be preferable to use new subcommander Classes, even if some text munging is required.

In any case, @c.command will likely morph into @g.commander_command.

Edward

[Edward K. Ream]

On Thursday, November 23, 2017 at 1:57:15 AM UTC-6, Edward K. Ream wrote:

> @c.command will likely morph into @g.commander_command.

Here is a slight variant of g.command that looks like it will suffice to define commands as "bare" functions using @g.commander_command:

class CommanderCommand(object):
    '''
    A global decorator for creating commander commands, that is, commands
    that were formerly methods of the Commands class in leoCommands.py.

    Usage:

        @g.command('command-name')
        def command_name(self, *args, **kwargs):
            ...
       
    The decorator injects command_name into the Commander class and calls
    funcToMethod so the ivar will be injected in all future commanders.

    g can *not* be used anywhere in this class!
    '''

    def __init__(self, name, **kwargs):
        '''Ctor for command decorator class.'''
        self.name = name

    def __call__(self, func):
        '''Register command for all future commanders.'''
        global_commands_dict[self.name] = func
        if app:
            # funcToMethod ensures the command will be
            # injected into all future commanders.
            import leo.core.leoCommands as leoCommands
            funcToMethod(func, leoCommands.Commands)
            for c in app.commanders():
                c.k.registerCommand(self.name, func)
        # Inject ivars for plugins_menu.py.
        func.is_command = True
        func.command_name = self.name
        return func

commander_command = CommanderCommand

To use this, we would just copy code from leoCommands.py to the new file, changing @g.command to g.commander_command.  Like this:

@g.commander_command('save')
def save(self, event=None, fileName=None):
    # Exactly the same code as before.

I'll play with this in a new cmds branch. I have high hopes that everything will "just work", with just a few tweaks to support code like k.simulateCommand. I'll abandon this project if it can't be completed in a day or three.

Edward

P.S. I don't think pyflakes has a right to complain about functions having "self" as the first argument, but I'll check first.

EKR

[Edward K. Ream]

On Thursday, November 23, 2017 at 8:41:57 AM UTC-6, Edward K. Ream wrote:

Here is a slight variant of g.command that looks like it will suffice to define commands as "bare" functions using @g.commander_command:

Here is a test (in a Leo script, not a file):

@others # Define commander_command

if not hasattr(c, 'save2'):

    @commander_command('save2')
    def save2(self, *args, **kwargs):
        g.trace('self', self, 'args', args, 'kwargs', kwargs)

c.save2()
c.k.simulateCommand('save2')

And here are the results, including a trace in k.simulateCommand:

save2 self Commander 149029168: 'C:\\Users\\edreamleo\\ekr.leo' args () kwargs {}
save2 self LeoKeyEvent:
{
    c: 'ekr.leo',
    char: '',
    event: None,
    stroke: None,
    w: None
} args () kwargs {}

This is why I say that tweaks will be needed to k.simulateCommand. In some cases it should pass k.c to the command handler rather than an event arg. I expect this can be done cleanly.  We shall see...

Edward

[Edward K. Ream]

On Thursday, November 23, 2017 at 8:50:57 AM UTC-6, Edward K. Ream wrote:

#589 Now tracks this project, and documents the cmds branch.

Preliminary tests are successful.  As expected, neither pylint nor pyflakes complains about top-level functions having a "self" arg.

Edward

[Edward K. Ream]

Recent revs in the cmds branch have successfully migrated all of the edit commands to leo/commands/commanderEditCommands.py.  I now have a much better idea of what is needed. Considerable "routine" code changes have been required, but unit tests have revealed all problems.

Imo, this project is clearly going to succeed.  Better, the work is worth doing for it's own sake.  In all cases, the required changes improve the clarity and encapsulation of the code.  A few more days may be needed, but this project now seems worth doing regardless of how long it takes.

Edward

[Terry Brown]

On Fri, 24 Nov 2017 00:50:21 -0800 (PST)
"Edward K. Ream" <edre...@gmail.com> wrote:

> Recent revs in the cmds branch have successfully migrated all of the
> edit commands to leo/commands/commanderEditCommands.py. I now have a
> much better idea of what is needed. Considerable "routine" code
> changes have been required, but unit tests have revealed all
> problems.
>
> Imo, this project is clearly going to succeed. Better, the work is
> worth doing for it's own sake. In all cases, the required changes
> improve the clarity and encapsulation of the code. A few more days
> may be needed, but this project now seems worth doing regardless of
> how long it takes.

Just want to check I understand this project.

Starting afresh, there'd be an outline class, Outline, commonly referred
to as 'o', with an ivar o.cmd, an instance of a Commands class,
defining commands that can be performed on an outline. That separation
doesn't exist, so the command definitions are being moved to a separate
file to reduce the size of the (current) Commands class. The Commands
class becomes more of an Outline class, but will still be called
Commands / c for everyones sanity (although I think there's the odd
place where c is described as a "context").

Am I even close? If I am, would a different approach (not that we
can't use the injection approach already taken), would be to move all
the implementations to a new OutlineCommands class, and then just point
to those from Commands, for backwards compatibility. so Commands.save()
becomes

@cmd('save-file')
def save(self, event=None, fileName=None):
self.outline_commands.save(self, event=None, fileName=None)

or perhaps even

save = some_function_that_acheives_the_above()

only because we need the backwards compatibility.

Just making sure I understand this very large change.

Cheers -Terry

[Edward K. Ream]

​​

On Fri, Nov 24, 2017 at 12:29 PM, Terry Brown <terry...@gmail.com> wrote:

Just want to check I understand this project.

​I've been playing with the project all day, trying to understand every little nuance. I believe the latest revs are just about perfect.

​> Starting afresh,

No, the project does not start afresh in any sense.  There are no new classes, no new subcommanders.  From a "live" objects point of view, almost nothing whatever has changed!

The only change to the init code is that c.initObjects must import the commander* modules so that decorators will be executed.

The only potential problem is that some of c's methods have become hidden helpers.  Mostly that's good (it cleans up c's namespace), but there is a slight possibility that some script might use one of these helpers.  If so, it will be easy to restore them. See below.

The @g.commander_command decorator is the key to everything.  In essence, it allows commander methods to be defined outside of leoCommands.py.  That's the only real difference in the code!  So the project is only about clever packaging.

The "proof" of this is that only simple changes have been made to the actual code. Helpers typically are no longer c methods, so their signature change to have a c arg instead of a self arg. The assignments c = self go away in these helpers.

As I have gone along, it has become clear which helpers should be hidden. These are just the helpers used internally by some command or other.  It's unlikely that outside code will ever know or care about these helpers. It would be no big deal if they need to become c methods again. Just:

- Move them back inside the Commands class.

- Change the signature to use self instead of c.

- Restore the assignment c = self.

- Have the calling code call c.x(...) instead of x(c, ...).

Almost all methods that define commands have migrated to the various commander*.py files.  Yes, there is some magic involved in injecting the functions into c, but the @g.commander_command decorator marks this magic explicitly.

All the remaining methods of the Commands class are either part of c's fundamental api, or are useful helpers. This wasn't apparent to me this morning.

I have spent several hours today organizing c's methods. The present categories are much much clearer than before.  Have a look at the organization of the Commands class!

Crucially, none of this reorganization affect the methods themselves in any way.  It only affects our understanding of the class.

​> ​

Just making sure I understand this very large change.

​Happily, the project is a small change to Leo's code.  Pyflakes, pylint and unit tests have caught all problems quickly.

I have been eating my own dog food without any problems whatever.  I expect only minor problems after the cmds branch merges into master.

Please feel free to ask more question. I don't plan any major changes to the code.  I am fairly shocked by how easy all this has been.

Edward

[Terry Brown]

On Fri, 24 Nov 2017 17:06:30 -0600

"Edward K. Ream" <edre...@gmail.com> wrote:

> ​> Starting afresh,
>
> No, the project does not start afresh in any sense. There are no new
> classes, no new subcommanders.

I didn't mean to imply that you had started afresh. I meant that if one
were to start afresh, one might arrange things as I went on to
describe, but constrained by the need for backwards compatibility, you
were trying to move in that direction by moving implementation to other
files.

I think it's clear enough what's been done. The value I think is in
the cleaning up of the namespace, rather than the reduction in size of
the definition of the Commands class. Or rather, the cleanup of the
namespace is a real improvement, while the reduction in size of the
definition of the Commands class kind of irrelevant, since Leo
eliminates any of the inconvenience of large source files.

Cheers -Terry

[Edward K. Ream]

On Sat, Nov 25, 2017 at 10:12 AM, Terry Brown <terry...@gmail.com> wrote:

I didn't mean to imply that you had started afresh. I meant that if one
were to start afresh, one might arrange things as I went on to
describe, but constrained by the need for backwards compatibility, you
were trying to move in that direction by moving implementation to other
files.

​Yes, that's correct.  Having commands be accessible​ via c is useful.  If I had it all over again, I don't think I would prefer c.fileCommands.save to c.save ;-)

the cleanup of the

​ ​

namespace is a real improvement

​Yes, it's unexpectedly important. Devs can now see that all the methods in the Commands class are there for a reason.

​

while the reduction in size of the

​ ​

definition of the Commands class kind of irrelevant, since Leo

​ ​

eliminates any of the inconvenience of large source files.

​I really like the new commander*.py files, partly because they are smaller (easier/faster to check via pylint) and partly because they are better encapsulation.

Even such a smallish thing as imports matter. It's now clear which imports each set of commands use. And they don't use many.

In short, this project is even more useful than I first imagined.

Edward