zsh-workers
 help / color / mirror / code / Atom feed
* Proof of concept mainstream plugin manager
@ 2016-01-22 19:44 Sebastian Gniazdowski
  2016-01-23  1:37 ` Bart Schaefer
  0 siblings, 1 reply; 23+ messages in thread
From: Sebastian Gniazdowski @ 2016-01-22 19:44 UTC (permalink / raw)
  To: Zsh hackers list

Hello
There are many plugin managers but the technology is not very
astonishing. The managers add any plugin to $fpath and that's pretty
it. FPATH gets flooded, possibly naive code gets injected into
session. That's why the technology isn't very popular on this mailing
list and on IRC.

I've utilized Bart's idea to prevent fpath from flooding:

  function some_function_from_plugin {
        local FPATH=/path/to/the/plugin
        autoload -X
    }

and written a plugin manager that utilizes this. Furthermore, any
autoload, bindkey, setopt is catched and a report of plugin operation
can be obtained:

https://asciinema.org/a/f4g9dgqcq2w7998761qyigqa2

I've written the proof of concept software in 1 hour. I wonder, could
this be a path that Zsh mainstream could take to provide robust
technology for plugins, whose users demand? 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

Best regards,
Sebastian Gniazdowski


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Proof of concept mainstream plugin manager
  2016-01-22 19:44 Proof of concept mainstream plugin manager Sebastian Gniazdowski
@ 2016-01-23  1:37 ` Bart Schaefer
  2016-01-23  9:03   ` Sebastian Gniazdowski
  0 siblings, 1 reply; 23+ messages in thread
From: Bart Schaefer @ 2016-01-23  1:37 UTC (permalink / raw)
  To: Zsh hackers list

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


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Proof of concept mainstream plugin manager
  2016-01-23  1:37 ` Bart Schaefer
@ 2016-01-23  9:03   ` Sebastian Gniazdowski
  2016-01-23 15:54     ` Sebastian Gniazdowski
  0 siblings, 1 reply; 23+ messages in thread
From: Sebastian Gniazdowski @ 2016-01-23  9:03 UTC (permalink / raw)
  To: Bart Schaefer; +Cc: Zsh hackers list

[-- Attachment #1: Type: text/plain, Size: 833 bytes --]

On 23 January 2016 at 02:37, Bart Schaefer <schaefer@brasslantern.com> wrote:
> As a final thought, I'd just call this whole thing "zplugin", forget the
> suffix "governor", name the variables ZPLUG_*, etc.

Renamed.

I occur a problem with traps. When zcurses waits for a key and Ctrl-C
is pressed, trap isn't called instantly, but later, in a case. I
attach test source. This is fixed in 5.1 (occurs in 5.0.8), wonder if
I can somehow workaround this:

https://asciinema.org/a/2dnx2ky1q14hna6bvwfrwsvkr

Discouraging, but quite expected. Doing autoload -X is expected to
generate some edge-cases on Zsh usage. If I could test say 30 plugins
from the awesome-zsh-plugins list, and confirm they're all working,
that would be an enabler for autoload -X, otherwise the project's
sense is doubtful.

Best regards,
Sebastian Gniazdowski

[-- Attachment #2: aload --]
[-- Type: application/octet-stream, Size: 448 bytes --]

zmodload zsh/curses

trap "echo Interrupted; return" TERM INT QUIT

zcurses init
zcurses delwin main 2>/dev/null
zcurses addwin main 4 $COLUMNS 0 0
zcurses border main
zcurses refresh main

a() {
    b="c"
    case "$a" in
        x*)
            echo "X"
            ;;
        *)
            echo "*"
            ;;
    esac
    sleep 20
}

# Waits for a key
zcurses input main key keypad

zcurses delwin main
zcurses end

set -x
a

# vim:ft=zsh

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Proof of concept mainstream plugin manager
  2016-01-23  9:03   ` Sebastian Gniazdowski
@ 2016-01-23 15:54     ` Sebastian Gniazdowski
  2016-01-23 16:00       ` Sebastian Gniazdowski
  0 siblings, 1 reply; 23+ messages in thread
From: Sebastian Gniazdowski @ 2016-01-23 15:54 UTC (permalink / raw)
  To: Bart Schaefer; +Cc: Zsh hackers list

On 23 January 2016 at 10:03, Sebastian Gniazdowski
<sgniazdowski@gmail.com> wrote:
> Discouraging, but quite expected. Doing autoload -X is expected to
> generate some edge-cases on Zsh usage. If I could test say 30 plugins
> from the awesome-zsh-plugins list, and confirm they're all working,
> that would be an enabler for autoload -X, otherwise the project's
> sense is doubtful.

Tried without eval:

    for i in "$@"; do
        $i() {
            local_FPATH
            builtin autoload -X
        }
        local replace_str="local
FPATH='$ZPLUGIN_HOME/plugins/${ZPLUGIN_CURRENT_USER}--${ZPLUGIN_CURRENT_PLUGIN}'"
        functions[$i]="${functions[$i]/local_FPATH/$replace_str}"
    done

https://github.com/psprint/zplugin/commit/8bcb689b5c808b7136f0385c9506fbaa3ef83f46

But the traps are still broken. That's a puzzle, as autoload does the
same thing – constructs function with autoload -X in its body...

Best regards,
Sebastian Gniazdowski


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Proof of concept mainstream plugin manager
  2016-01-23 15:54     ` Sebastian Gniazdowski
@ 2016-01-23 16:00       ` Sebastian Gniazdowski
  2016-01-23 16:09         ` Sebastian Gniazdowski
  0 siblings, 1 reply; 23+ messages in thread
From: Sebastian Gniazdowski @ 2016-01-23 16:00 UTC (permalink / raw)
  To: Bart Schaefer; +Cc: Zsh hackers list

Even this doesn't work:

    for i in "$@"; do
        fpath+=$ZPLUGIN_HOME/plugins/${ZPLUGIN_CURRENT_USER}--${ZPLUGIN_CURRENT_PLUGIN}
        $i() {
            builtin autoload -X
        }
    done

Looks like Zsh is better off with autoload-X-bodies created with
genuine autoload. But that's a puzzle as it's still the same function
bod

Best regards,
Sebastian Gniazdowski


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Proof of concept mainstream plugin manager
  2016-01-23 16:00       ` Sebastian Gniazdowski
@ 2016-01-23 16:09         ` Sebastian Gniazdowski
  2016-01-23 16:26           ` Sebastian Gniazdowski
  2016-01-23 17:36           ` Bart Schaefer
  0 siblings, 2 replies; 23+ messages in thread
From: Sebastian Gniazdowski @ 2016-01-23 16:09 UTC (permalink / raw)
  To: Bart Schaefer; +Cc: Zsh hackers list

And even this doesn't work:

    for i in "$@"; do
        functions[$i]="
            local
FPATH='$ZPLUGIN_HOME/plugins/${ZPLUGIN_CURRENT_USER}--${ZPLUGIN_CURRENT_PLUGIN}'
            builtin autoload -X
        "
    done

One hint is that when functions[$i] is called for second time, then
traps work fine

PS. Sorry for streams of consciousness, that should be the last
email... However at least each of the emails sent is constructive

Best regards,
Sebastian Gniazdowski


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Proof of concept mainstream plugin manager
  2016-01-23 16:09         ` Sebastian Gniazdowski
@ 2016-01-23 16:26           ` Sebastian Gniazdowski
  2016-01-23 17:36           ` Bart Schaefer
  1 sibling, 0 replies; 23+ messages in thread
From: Sebastian Gniazdowski @ 2016-01-23 16:26 UTC (permalink / raw)
  To: Bart Schaefer; +Cc: Zsh hackers list

One final trick:

    for i in "$@"; do
        fpath+=$ZPLUGIN_HOME/plugins/${ZPLUGIN_CURRENT_USER}--${ZPLUGIN_CURRENT_PLUGIN}
        autoload "$i"
*        functions[$i]=$functions[$i]
    done

Without the * line traps work fine, with it – a deferred invocation of
Ctrl-C in wild-carded case occurs. Seems like autoload -X doesn't like
to be put into function's body in any other way than via autoload.

Best regards,
Sebastian Gniazdowski


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Proof of concept mainstream plugin manager
  2016-01-23 16:09         ` Sebastian Gniazdowski
  2016-01-23 16:26           ` Sebastian Gniazdowski
@ 2016-01-23 17:36           ` Bart Schaefer
  2016-01-23 19:20             ` Bart Schaefer
  1 sibling, 1 reply; 23+ messages in thread
From: Bart Schaefer @ 2016-01-23 17:36 UTC (permalink / raw)
  To: Zsh hackers list

On Jan 23, 10:03am, Sebastian Gniazdowski wrote:
}
} I occur a problem with traps.

Hm.  The difference between a "real" autolaod and a function that calls
"autoload -X" is a couple levels of function stack depth -- in the "real"
case zsh knows it can optimize out the first call:

torch% functions showfunctrace
torch% autoload !$
autoload showfunctrace
torch% showfunctrace
Src/zsh:30
torch% unfunction showfunctrace
torch% showfunctrace() { autoload -X }
torch% showfunctrace
(eval):1
showfunctrace:0
Src/zsh:33
torch% 

This will affect traps created with the trap command because they
execute in the same scope as the function call that defined them,
rather than "inside" the call that defined them; so some convoluted
trap tricks that rely on the scope may not work.

} This is fixed in 5.1 (occurs in 5.0.8), wonder if
} I can somehow workaround this:

Probably not, it's almost certainly an internal signal handling thing.

There's another problem with the "local FPATH" trick that I thought
of just now:  It changes the load path for all other functions down
the call stack, which is likely to cause some other autoloads to fail.
A better approach is a temporary prepend.

    local FPATH="$ZPLUGIN_HOME/plugins/${ZPLUGIN_CURRENT_USER}--${ZPLUGIN_CURRENT_PLUGIN}:$FPATH"

A less pleasing approach to the whole thing is to use "autoload +X"
instead:

  preload-autoload() {
    FPATH="/tmp:$FPATH" autoload +X $1
  }

This causes the function to be loaded immediately but not run.  So it
"fixes" the trap problem but fills up memory with functions that may
never be used.


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Proof of concept mainstream plugin manager
  2016-01-23 17:36           ` Bart Schaefer
@ 2016-01-23 19:20             ` Bart Schaefer
  2016-01-23 20:00               ` Bart Schaefer
  0 siblings, 1 reply; 23+ messages in thread
From: Bart Schaefer @ 2016-01-23 19:20 UTC (permalink / raw)
  To: Zsh hackers list

On Jan 23,  9:36am, Bart Schaefer wrote:
}
} A less pleasing approach to the whole thing is to use "autoload +X"

Another possibly is this, though it still doesn't address all potential
issues with overly-clever uses of "trap":

 shadow-autoload() {
   local -a opts
   local func
   # zparseotps doesn't handle the "+" form, sigh;
   # here as placeholder for something more thorough
   zparseopts -a opts -D ${(s::):-TUXkmtzw}
   if (( $opts[(I)-X] )); then
     print -u2 "builtin autoload required for -X"
     return 1
   fi
   if (( $opts[(I)-w] )); then
     builtin autoload $opts "$@"
     return
   fi
   for func; do
     eval "function $func {
       unfunction $func
       local FPATH=${(q)PLUGIN_DIR}"'":${FPATH}"'"
       autoload $opts $func
       $func
     }"
   done
 }

This seems to fix the problems with your "aload" test case.  Some old
versions of zsh may have problems with a function that unfunctions
itself, but they're probably old enough not to matter.


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Proof of concept mainstream plugin manager
  2016-01-23 19:20             ` Bart Schaefer
@ 2016-01-23 20:00               ` Bart Schaefer
  2016-01-24 10:51                 ` Sebastian Gniazdowski
  2016-01-26 22:50                 ` Daniel Shahaf
  0 siblings, 2 replies; 23+ messages in thread
From: Bart Schaefer @ 2016-01-23 20:00 UTC (permalink / raw)
  To: Zsh hackers list

On Jan 23, 11:20am, I wrote:
}
}      eval "function $func {
}        unfunction $func
}        local FPATH=${(q)PLUGIN_DIR}"'":${FPATH}"'"
}        autoload $opts $func
}        $func
}      }"

The last line of course needs to be 

    $func "$@"

If that's the approach, it might be useful to do something like this:

    _reload_and_run () {
      local fpath_prefix="$1" autoload_opts="$2" func="$3"
      shift 3
      unfunction "$func"
      local FPATH="$fpath_prefix":"${FPATH}"
      builtin autoload $=autoload_opts "$func"
      "$func" "$@"
    }

    shadow-autoload () {
      local -a opts
      local func
      zparseopts -a opts -D ${(s::):-TUXkmtzw}
      if (( $opts[(I)(-|+)X] ))
      then
        print -u2 "builtin autoload required for $opts"
        return 1
      fi
      if (( $opts[(I)-w] ))
      then
        builtin autoload $opts "$@"
        return
      fi
      for func
      do
        eval "function $func { 
          _reload_and_run ${(q)PLUGIN_DIR} ${(qq)opts} $func "'"$@"
        }'
      done
    }


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Proof of concept mainstream plugin manager
  2016-01-23 20:00               ` Bart Schaefer
@ 2016-01-24 10:51                 ` Sebastian Gniazdowski
  2016-01-24 14:59                   ` Sebastian Gniazdowski
  2016-01-26 22:50                   ` Daniel Shahaf
  2016-01-26 22:50                 ` Daniel Shahaf
  1 sibling, 2 replies; 23+ messages in thread
From: Sebastian Gniazdowski @ 2016-01-24 10:51 UTC (permalink / raw)
  To: Bart Schaefer; +Cc: Zsh hackers list

On 23 January 2016 at 21:00, Bart Schaefer <schaefer@brasslantern.com> wrote:

> If that's the approach, it might be useful to do something like this:
>
>     _reload_and_run () {

The code works very well, which is extremely good, encouraging news. I
will now move to test ~30 plugins from awesome-zsh-plugins github
page, at the moment I tested my 3 plugins which are quite fancy (they
use traps for example), and everything works fine.

I've updated zplugin and commited your code, to the project that's
under MIT license. Is this fine? Maybe you would want to participate
directly? Or you Daniel? I imagine that currently the project is at
rapid iterations stage and quick direct commits would be good.

Changes I've made:
- shadowed zstyle
- reports are now gathered per-plugin, in associative array containing
strings with \n in them
- typeset -gH
- colors
- shadowing functions start with "-", private functions with "_", user
accesible functions with no prefix (however, we would probably want
only one user accessible function – "zplugin" – with sub-commands such
as load, show-report, etc.)

Here is how it currently works:

https://asciinema.org/a/753uyr5uv0sphp4y9srq59vcv

>         eval "function $func {
>           _reload_and_run ${(q)PLUGIN_DIR} ${(qq)opts} $func "'"$@"
>         }'

Changed the eval to this, not sure if eval here was any risk, but this
also looks very fine:

        functions[$func]="-zplugin_reload_and_run ${(q)PLUGIN_DIR}
${(qq)opts} $func "'"$@"'

Source is at:

https://github.com/psprint/zplugin

Best regards,
Sebastian Gniazdowski


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Proof of concept mainstream plugin manager
  2016-01-24 10:51                 ` Sebastian Gniazdowski
@ 2016-01-24 14:59                   ` Sebastian Gniazdowski
  2016-01-24 19:06                     ` Sebastian Gniazdowski
  2016-01-24 20:45                     ` Bart Schaefer
  2016-01-26 22:50                   ` Daniel Shahaf
  1 sibling, 2 replies; 23+ messages in thread
From: Sebastian Gniazdowski @ 2016-01-24 14:59 UTC (permalink / raw)
  To: Bart Schaefer; +Cc: Zsh hackers list

Tested 8 plugins:

horosgrisa/autoenv
adolfoabegg/browse-commit
arzzen/calc.plugin.zsh
walesmd/caniuse.plugin.zsh
mollifier/cd-gitroot
mikedacre/cdbk
willghatch/zsh-cdr
rutchkiwi/copyzshell (requires OMZ directory structure simulation)

And they all work. Except maybe for zsh-cdr, which has the following
glitch: when I run my n-cd, select directory, which will cause "cd" to
be called, then _reload_and_run will cause error on line:


"    builtin autoload $=autoload_opts "$func""

That's very weird, as when I display the options right before the line:

    echo "Running with opts: " $=autoload_opts ", for func: $func"

then there is no improper option in $autoload_opts. And, second call
to n-cd works fine... I can workaround this by doing "command cd" in
my tool instead of just "cd", but still, that's a puzzle, what can be
going on? Decided to write about this and move onward testing next
plugins. Here is movie that shows the weird behavior:

https://asciinema.org/a/bn8b73y8t308qimrrxmvxqlc2

Where in Zsh source does "autload" report anything about option "-" ?
Maybe I could apply some patch and build Zsh to see some clues?

PS. ZPlugin proves to be very nice. Very nice to see what plugins do:

https://asciinema.org/a/dtgdzr4wqe9nizreqjlgrjmik

It also confirms that the plugins contain rather naive code, like
after reading Advanced Bash Scripting Guide, however, they rather just
work, like the autoenv plugin. I'm thinking about reporting that
plugin does use binaries like grep or sed, which would give a hint of
how naive the code loaded is.

Best regards,
Sebastian Gniazdowski


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Proof of concept mainstream plugin manager
  2016-01-24 14:59                   ` Sebastian Gniazdowski
@ 2016-01-24 19:06                     ` Sebastian Gniazdowski
  2016-01-24 20:45                     ` Bart Schaefer
  1 sibling, 0 replies; 23+ messages in thread
From: Sebastian Gniazdowski @ 2016-01-24 19:06 UTC (permalink / raw)
  To: Bart Schaefer; +Cc: Zsh hackers list

Tested 21 plugins:

horosgrisa/autoenv
adolfoabegg/browse-commit
arzzen/calc.plugin.zsh
walesmd/caniuse.plugin.zsh
mollifier/cd-gitroot
MikeDacre/cdbk
willghatch/zsh-cdr
rutchkiwi/copyzshell (required OMZ directory structure simulation)
veelenga/crystal-zsh
jgrowl/depot_tools
gusaiani/elixir-oh-my-zsh
b4b4r07/emoji-cli
b4b4r07/enhancd
uvaes/fzf-marks
voronkovich/get-jquery.plugin.zsh
fontno/ghost_zeus
peterhurford/git-aliases.zsh
unixorn/git-extra-commands
peterhurford/git-it-on.zsh
tevren/gitfast-zsh-plugin

Only the last one doesn't work, but it's for sure broken, that's not
zplugin's fault. So currently only one small problem exists, the
glitch between zsh-navigation-tools/n-cd and zsh-cdr.

Added shadowing of `alias' and `zle' and `source' reporting, will now
think about doing diff on $functions before and after a plugin is
load, to report what functions were created by the plugin, then I will
test remaining plugins

Very nice reports at the end:
https://asciinema.org/a/6hem25k7yls8otgtilkf7idsw

Best regards,
Sebastian Gniazdowski


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Proof of concept mainstream plugin manager
  2016-01-24 14:59                   ` Sebastian Gniazdowski
  2016-01-24 19:06                     ` Sebastian Gniazdowski
@ 2016-01-24 20:45                     ` Bart Schaefer
  2016-06-04 11:36                       ` Sebastian Gniazdowski
  1 sibling, 1 reply; 23+ messages in thread
From: Bart Schaefer @ 2016-01-24 20:45 UTC (permalink / raw)
  To: Zsh hackers list

On Jan 24,  3:59pm, Sebastian Gniazdowski wrote:
}
} Where in Zsh source does "autload" report anything about option "-" ?

This is happening because you removed the "eval" and replaced it with
assignment to functions[$func].  ${(qq)opts} is no longer the correct
quoting in that formulation, in fact you don't need any quoting of
$opts at all.   The effect of (qq) is that you are passing "-U -z" as
a single string, not as two separate arguments.  (I'm surprised there
wasn't a complaint about space being an unknown option as well.)

} Maybe I could apply some patch and build Zsh to see some clues?

You could do 
    functions -t -- -zplugin-shadow-autoload
to get xtrace output for just that one function.


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Proof of concept mainstream plugin manager
  2016-01-24 10:51                 ` Sebastian Gniazdowski
  2016-01-24 14:59                   ` Sebastian Gniazdowski
@ 2016-01-26 22:50                   ` Daniel Shahaf
  2016-01-27  7:47                     ` Sebastian Gniazdowski
  1 sibling, 1 reply; 23+ messages in thread
From: Daniel Shahaf @ 2016-01-26 22:50 UTC (permalink / raw)
  To: Sebastian Gniazdowski; +Cc: Zsh hackers list

Sebastian Gniazdowski wrote on Sun, Jan 24, 2016 at 11:51:49 +0100:
> I've updated zplugin and commited your code, to the project that's
> under MIT license. Is this fine? Maybe you would want to participate
> directly? Or you Daniel? I imagine that currently the project is at
> rapid iterations stage and quick direct commits would be good.

Thanks for the invite, but since I don't use a plugin manager, I don't
expect to contribute much.  I'd be more interested in the "How to
package a plugin to make it loadable by zplugin" side of things.

Cheers,

Daniel


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Proof of concept mainstream plugin manager
  2016-01-23 20:00               ` Bart Schaefer
  2016-01-24 10:51                 ` Sebastian Gniazdowski
@ 2016-01-26 22:50                 ` Daniel Shahaf
  2016-01-27  4:34                   ` Bart Schaefer
  1 sibling, 1 reply; 23+ messages in thread
From: Daniel Shahaf @ 2016-01-26 22:50 UTC (permalink / raw)
  To: Zsh hackers list

Bart Schaefer wrote on Sat, Jan 23, 2016 at 12:00:55 -0800:
>         eval "function $func { 
>           _reload_and_run ${(q)PLUGIN_DIR} ${(qq)opts} $func "'"$@"
>         }'

Perhaps change $func to ${(q)func}.


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Proof of concept mainstream plugin manager
  2016-01-26 22:50                 ` Daniel Shahaf
@ 2016-01-27  4:34                   ` Bart Schaefer
  2016-01-27  7:34                     ` Sebastian Gniazdowski
  0 siblings, 1 reply; 23+ messages in thread
From: Bart Schaefer @ 2016-01-27  4:34 UTC (permalink / raw)
  To: Zsh hackers list

On Jan 26, 10:50pm, Daniel Shahaf wrote:
} Subject: Re: Proof of concept mainstream plugin manager
}
} Bart Schaefer wrote on Sat, Jan 23, 2016 at 12:00:55 -0800:
} >         eval "function $func { 
} >           _reload_and_run ${(q)PLUGIN_DIR} ${(qq)opts} $func "'"$@"
} >         }'
} 
} Perhaps change $func to ${(q)func}.

Sebastian has already changed that whole thing to be

functions[$func]="_reload_and_run ${(q)PLUGIN_DIR} $opts $func "'"$@"

which yes, is going to do strange things if $func contains spaces.

On the other hand, the whole plugin would already be broken if $func
contains spaces.  I'd rather not "fix" it in an unpredictable way.


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Proof of concept mainstream plugin manager
  2016-01-27  4:34                   ` Bart Schaefer
@ 2016-01-27  7:34                     ` Sebastian Gniazdowski
  2016-01-28  6:38                       ` Bart Schaefer
  0 siblings, 1 reply; 23+ messages in thread
From: Sebastian Gniazdowski @ 2016-01-27  7:34 UTC (permalink / raw)
  To: Bart Schaefer; +Cc: Zsh hackers list

On 27 January 2016 at 05:34, Bart Schaefer <schaefer@brasslantern.com> wrote:
> On Jan 26, 10:50pm, Daniel Shahaf wrote:
> } Subject: Re: Proof of concept mainstream plugin manager
> }
> } Bart Schaefer wrote on Sat, Jan 23, 2016 at 12:00:55 -0800:
> } >         eval "function $func {
> } >           _reload_and_run ${(q)PLUGIN_DIR} ${(qq)opts} $func "'"$@"
> } >         }'
> }
> } Perhaps change $func to ${(q)func}.
>
> Sebastian has already changed that whole thing to be
>
> functions[$func]="_reload_and_run ${(q)PLUGIN_DIR} $opts $func "'"$@"

I commited your eval way to recognize it in battle (Polish idiom,
wonder if it works in English). Without quoting in ${(qq)opts}, what
you recommended earlier, I get list of all defined functions and their
bodies when shell starts (code visible in 27 sec). I tried this before
but didn't want to bother you because it's hard to express what's
happening, thus the video:

https://asciinema.org/a/eellh7umed2z45685tlhqgdqp

However, suddenly the glitch between ncd and zsh-cdr is gone. Don't
know what's happening, it was there yesterday. I guess it will be
back.

> which yes, is going to do strange things if $func contains spaces.
>
> On the other hand, the whole plugin would already be broken if $func
> contains spaces.  I'd rather not "fix" it in an unpredictable way.

What do you mean? Shouldn't I do quoting, i.e. ${(q)func} ?

Best regards,
Sebastian Gniazdowski


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Proof of concept mainstream plugin manager
  2016-01-26 22:50                   ` Daniel Shahaf
@ 2016-01-27  7:47                     ` Sebastian Gniazdowski
  0 siblings, 0 replies; 23+ messages in thread
From: Sebastian Gniazdowski @ 2016-01-27  7:47 UTC (permalink / raw)
  To: Daniel Shahaf; +Cc: Zsh hackers list

On 26 January 2016 at 23:50, Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
> Sebastian Gniazdowski wrote on Sun, Jan 24, 2016 at 11:51:49 +0100:
> I'd be more interested in the "How to
> package a plugin to make it loadable by zplugin" side of things.

    matches=(
        $dname/$pdir/init.zsh(N) $dname/${pdir}.plugin.zsh(N)
        $dname/${pdir}.zsh-theme(N) $dname/${pdir}.theme.zsh(N)
        $dname/${pdir}.zshplugin(N) $dname/${pdir}.zsh.plugin(N)
        $dname/*.plugin.zsh(N) $dname/*.zsh(N) $dname/*.sh(N)
    )
    [ "$#matches" -eq "0" ] && return 1
    local fname="${matches[1]#$dname/}"

That's how things are done currently. This is based on what zgen
checks for, but done better, I quess, in single line. So, to create a
plugin, a directory with e.g. "directoryname.plugin.zsh" is needed,
and then it can do autoloads and setopts, etc. Zplugin does more than
just sourcing, tracks e.g. the setopts, to report them and also to
allow unloading, with options restoration.

Best regards,
Sebastian Gniazdowski


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Proof of concept mainstream plugin manager
  2016-01-27  7:34                     ` Sebastian Gniazdowski
@ 2016-01-28  6:38                       ` Bart Schaefer
  2016-01-28  7:13                         ` Sebastian Gniazdowski
  0 siblings, 1 reply; 23+ messages in thread
From: Bart Schaefer @ 2016-01-28  6:38 UTC (permalink / raw)
  To: Zsh hackers list

On Jan 27,  8:34am, Sebastian Gniazdowski wrote:
} Subject: Re: Proof of concept mainstream plugin manager
}
} > Sebastian has already changed that whole thing to be
} >
} > functions[$func]="_reload_and_run ${(q)PLUGIN_DIR} $opts $func "'"$@"
} 
} I commited your eval way to recognize it in battle (Polish idiom,
} wonder if it works in English). Without quoting in ${(qq)opts}, what
} you recommended earlier, I get list of all defined functions

Oh.  Yes, you need to pass empty string there if there are no options,
because _reload_and_run interprets its first 3 arguments positionally
and all are required to be present.  Sorry about that.

So if you want to use the assignment form, you need

functions[$func]="_reload_and_run ${(q)PLUGIN_DIR} ${${(q)opts[@]}} $func "'"$@"'

The ${(qq)opts[@]} quotes every element separately, and then the outer
${...} compbines the result into a single string again.

} > which yes, is going to do strange things if $func contains spaces.
} >
} > On the other hand, the whole plugin would already be broken if $func
} > contains spaces.  I'd rather not "fix" it in an unpredictable way.
} 
} What do you mean? Shouldn't I do quoting, i.e. ${(q)func} ?

Whether you want the (q) is entirely up to you.  The result will be
broken if there are any spaces in $func whether you (q) or not,
because functions[$func]=... will already have created something
that can't be run.

Back on the first hand, I suppose if $func somehow contains a semicolon
or some globbing characters, rather than just spaces, then you should
quote those.  So yes, ${(q)func} is better.


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Proof of concept mainstream plugin manager
  2016-01-28  6:38                       ` Bart Schaefer
@ 2016-01-28  7:13                         ` Sebastian Gniazdowski
  0 siblings, 0 replies; 23+ messages in thread
From: Sebastian Gniazdowski @ 2016-01-28  7:13 UTC (permalink / raw)
  To: Bart Schaefer; +Cc: Zsh hackers list

On 28 January 2016 at 07:38, Bart Schaefer <schaefer@brasslantern.com> wrote:
> Oh.  Yes, you need to pass empty string there if there are no options,
> because _reload_and_run interprets its first 3 arguments positionally
> and all are required to be present.  Sorry about that.
>
> So if you want to use the assignment form, you need
>
> functions[$func]="_reload_and_run ${(q)PLUGIN_DIR} ${${(q)opts[@]}} $func "'"$@"'
>
> The ${(qq)opts[@]} quotes every element separately, and then the outer
> ${...} compbines the result into a single string again.

I still get the same behavior

> Back on the first hand, I suppose if $func somehow contains a semicolon
> or some globbing characters, rather than just spaces, then you should
> quote those.  So yes, ${(q)func} is better.

Did one test. There is one simple plugin calc, which does:

function =
{
  echo "$@" | bc -l
}

If I convert this into autoload function it doesn't break without (q)
in ${(q)func}. That said, I'll better stick with (q).

Best regards,
Sebastian Gniazdowski


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Proof of concept mainstream plugin manager
  2016-01-24 20:45                     ` Bart Schaefer
@ 2016-06-04 11:36                       ` Sebastian Gniazdowski
  2016-06-04 17:02                         ` Bart Schaefer
  0 siblings, 1 reply; 23+ messages in thread
From: Sebastian Gniazdowski @ 2016-06-04 11:36 UTC (permalink / raw)
  To: Bart Schaefer; +Cc: Zsh hackers list

On 24 January 2016 at 21:45, Bart Schaefer <schaefer@brasslantern.com> wrote:
> On Jan 24,  3:59pm, Sebastian Gniazdowski wrote:
> }
> } Where in Zsh source does "autload" report anything about option "-" ?
>
> This is happening because you removed the "eval" and replaced it with
> assignment to functions[$func].  ${(qq)opts} is no longer the correct
> quoting in that formulation, in fact you don't need any quoting of
> $opts at all.   The effect of (qq) is that you are passing "-U -z" as
> a single string, not as two separate arguments.

This came back and now finally resolved. Things work in plugin's
context when auto-loaded function is called. And plugin did local
IFS=$'\n'. So this line in --zplg-reload-and-run:

     builtin autoload $=autoload_opts "$func"

wasn't splitting $autoload_opts on space :)

Best regards,
Sebastian Gniazdowski


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Proof of concept mainstream plugin manager
  2016-06-04 11:36                       ` Sebastian Gniazdowski
@ 2016-06-04 17:02                         ` Bart Schaefer
  0 siblings, 0 replies; 23+ messages in thread
From: Bart Schaefer @ 2016-06-04 17:02 UTC (permalink / raw)
  To: Zsh hackers list

On Jun 4,  1:36pm, Sebastian Gniazdowski wrote:
}
} plugin did local IFS=$'\n'. So this line in --zplg-reload-and-run:
} 
}      builtin autoload $=autoload_opts "$func"
} 
} wasn't splitting $autoload_opts on space :)

Urk.  So you need to locally reset IFS to something sane, or else use
${(s: :)autoload_opts}.

There appear to be about 30 places in the standard zsh function suite
where a local change to $IFS could similarly break things.

I'm willing to bet that the plugin has scoped that $IFS change a bit
more widely than where it is really needed.


^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2016-06-04 17:02 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-01-22 19:44 Proof of concept mainstream plugin manager Sebastian Gniazdowski
2016-01-23  1:37 ` Bart Schaefer
2016-01-23  9:03   ` Sebastian Gniazdowski
2016-01-23 15:54     ` Sebastian Gniazdowski
2016-01-23 16:00       ` Sebastian Gniazdowski
2016-01-23 16:09         ` Sebastian Gniazdowski
2016-01-23 16:26           ` Sebastian Gniazdowski
2016-01-23 17:36           ` Bart Schaefer
2016-01-23 19:20             ` Bart Schaefer
2016-01-23 20:00               ` Bart Schaefer
2016-01-24 10:51                 ` Sebastian Gniazdowski
2016-01-24 14:59                   ` Sebastian Gniazdowski
2016-01-24 19:06                     ` Sebastian Gniazdowski
2016-01-24 20:45                     ` Bart Schaefer
2016-06-04 11:36                       ` Sebastian Gniazdowski
2016-06-04 17:02                         ` Bart Schaefer
2016-01-26 22:50                   ` Daniel Shahaf
2016-01-27  7:47                     ` Sebastian Gniazdowski
2016-01-26 22:50                 ` Daniel Shahaf
2016-01-27  4:34                   ` Bart Schaefer
2016-01-27  7:34                     ` Sebastian Gniazdowski
2016-01-28  6:38                       ` Bart Schaefer
2016-01-28  7:13                         ` Sebastian Gniazdowski

Code repositories for project(s) associated with this public inbox

	https://git.vuxu.org/mirror/zsh/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).