Mailing-List: contact zsh-workers-help@zsh.org; run by ezmlm
Precedence: bulk
From: Bart Schaefer <schaefer@brasslantern.com>
Message-Id: <160122173705.ZM11491@torch.brasslantern.com>
Date: Fri, 22 Jan 2016 17:37:05 -0800
In-Reply-To: <CAKc7PVByfo51GcVkb_kYgO2D3cWYrcq-p4v8gmyfsaeWU=2new@mail.gmail.com>
Comments: In reply to Sebastian Gniazdowski <sgniazdowski@gmail.com>
        "Proof of concept mainstream plugin manager" (Jan 22,  8:44pm)
References: <CAKc7PVByfo51GcVkb_kYgO2D3cWYrcq-p4v8gmyfsaeWU=2new@mail.gmail.com>
To: Zsh hackers list <zsh-workers@zsh.org>
Subject: Re: Proof of concept mainstream plugin manager
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii

On Jan 22,  8:44pm, Sebastian Gniazdowski wrote:
}
} I imagine much more can be done if mainstream pairs of eyes will look
} at this. Any ideas of what can be done?
} 
} https://github.com/psprint/zsh-plugin-governor

Some thoughts ...

Consider declaring some or all ZGOV_* variables with "typeset -gH".

For shadowing commands like "autoload", you'd be better off using
function wrappers rather than aliases.  There are too many ways that
aliases can be turned off or bypassed, and none of the commands you
are replacing have keyword-like behavior.

I'm not sure how much value there is in capturing "setopt".  It's
really difficult to manage the side-effects of the "localoptions"
option within a wrapper function, and there are several other ways
to change options as well.  

You're failing to handle any of the flags of the "autoload" command
in zgov-shadow-autoload.  You can probably pass these through to the
"builtin autoload -X" inside the "eval".

You can of course eval a single double-quoted string with embedded
newlines instead of ending all those lines with backslash.

Various things in here are going to gripe if the warn_create_global
option is set.  Hard to decide what to do about that given that this
file is meant to be loaded with "source".  Perhaps wrap the entire
file in an anonymous function:

    () {
      emulate -LR zsh
      unsetopt warncreateglobal # not needed as of 5.3 with emulate

      # your entire current file contents go here
      # except you must use explicit "typeset -g"
    }

Every plugin that you "source" from inside zgov-load-plugin is going
to have a similar problem:  They're one level deeper in the function
stack than they would be if sourced directly from .zshrc and need to
be prepared for that.

For unloading, what you need is a diff of ${(k)functions} before and
after the plugin is loaded, assuming that the plugin really does
autoload or otherwise declare all its function names at load time.
Possibly a conventional unload function name, expected to be defined
by the plugin, could be called if it is in fact defined.  You may even
want the sort of setup/boot/cleanup/finish model used by zmodload
for binary object modules.

As a final thought, I'd just call this whole thing "zplugin", forget the
suffix "governor", name the variables ZPLUG_*, etc.

-- 
Barton E. Schaefer