Fish-like autosuggestions

Fish-like autosuggestions

[Thiago Padilha]

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

Hi

I've just finished implementing a zle widget that provides fish-like
fast/unobtrusive autosuggestions.

Like the 'predict-on' widget, this one uses suggestions from from history
then from the user completion functions. There are a couple of differences
though:

- Only the part left of the cursor is considered(the rest is 'grayed-out'
to give a visual hint). The suggested part must be accepted explicitly
- For suggesting with user completion functions, a suggestion daemon(shared
by all shells for an user) is used. The suggestion is updated
asynchronously with zle -F so the shell will be responsive even with slow
completion functions.
- The widget is 'paused' when doing certain actions that could lead to
editing the middle of the line(eg entering vi mode or moving the cursor to
the left)

I'm sure there are many improvements to be made as I am not a zsh expert.
One thing I couldnt figure out is how to handle ctrl+c correctly: since
recursive-edit is used ctrl+c has to be pressed two times to exit zle

I'm also sure there are many bugs, but right now the widget should be
usable enough, so feel free to try and give feedback :)

For anyone interested, here it is:
https://github.com/tarruda/zsh-autosuggestions (I recommend trying, this
will likely improve efficiency in using zsh history)

The daemon uses valodim's code to extract completions programmatically:
https://github.com/Valodim/zsh-capture-completion , so big thanks to him

Re: Fish-like autosuggestions

[Bart Schaefer]

On Oct 29,  3:52pm, Thiago Padilha wrote:
}
} I'm sure there are many improvements to be made as I am not a zsh expert.

I have some concerns about the client/daemon model (for example what
prevents a malicious program from connecting to the daemon and sending
input that would cause the zpty shell to execute a command?) but I
haven't studied it very closely.

Since you're already doing "zle recursive-edit", you might want to look
into creating your own keymap instead of changing widget bindings in
the main keymap.  There are some examples and helper code for this in
Functions/Zle/keymap+widget in the distribution.

Re: Fish-like autosuggestions

[Thiago Padilha]

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

On Wed, Oct 30, 2013 at 1:25 PM, Bart Schaefer <schaefer@brasslantern.com>wrote:

> I have some concerns about the client/daemon model (for example what
> prevents a malicious program from connecting to the daemon and sending
> input that would cause the zpty shell to execute a command?) but I
> haven't studied it very closely.
>

I dont understand how a zpty instance could be accessed from outside the
process that started it(I'm assuming its not accessible) but it shouldn't
be possible run commands as the daemon never sends any input that could
cause 'accept-line' to be run. I would also use the following arguments to
defend the security of the model:

1 - There's one daemon per user and the socket is writable only by the user
that started the daemon
2 - The socket lives in a user-restricted (700) temporary directory
3 - If there's malicious code running in the user context, why would it
need to connect to the daemon or a pty in order to damage the system?


> Since you're already doing "zle recursive-edit", you might want to look
> into creating your own keymap instead of changing widget bindings in
> the main keymap.  There are some examples and helper code for this in
> Functions/Zle/keymap+widget in the distribution.
>

I've been playing with keymap+widget but I'm still deciding whether it is
the best solution for the following reasons:

- I'm only using recusive-edit because I couldn't do asynchronous updates
to the zle RBUFFER using 'zle -F'(I started from the 'predict-on' code)
- Sometimes I have to deactivate the autosuggestions feature and
consequently some of the widget hooks(eg: editing the middle of the line or
in vi normal mode)

With the current issues in mind I have a few more questions:

- Is it possible to update the zle RBUFFER from outside a widget? The hook
set with 'zle -F' was being executed outside recursive-edit but it couldnt
modify the RBUFFER variable in that case. In the IRC channel Valodim
mentioned that the hook was not being executed inside zle's context. If so,
is there a way to enter zle's context from a hook set with 'zle -F' ?
- Can the keymap+widget feature be used outside recursive-edit? From what I
understood it will be in effect whenever a [keymap]+[widget] function is
defined and [keymap] is active, if thats the case I can simply switch
keymaps with 'zle -K [keymap]' whenever I need a set of widget hooks to be
active right?
- Whats the best way to modify RBUFFER before a completion widget is
executed? I've tried the comppostfuncs but the RBUFFER variable is
read-only in that context.

Re: Fish-like autosuggestions

[Bart Schaefer]

On Nov 4,  5:30pm, Thiago Padilha wrote:
}
} I would also use the following arguments to
} defend the security of the model:

Thanks for the summary; as I said I handn't looked closely.  I was thinking
of the doc for tcp_proxy:

     caution should be taken as there is no security whatsoever and
     this can leave your computer open to the world.  Ideally, it
     should only be used behind a firewall.

} I've been playing with keymap+widget but I'm still deciding whether it is
} the best solution for the following reasons:
} 
} - I'm only using recusive-edit because I couldn't do asynchronous updates
} to the zle RBUFFER using 'zle -F'(I started from the 'predict-on' code)

Hmm; so the trick here is that if you start recursive-edit, then when
the zle -F handler executes the special parameters from the surrounding
(original) ZLE context are still defined in the handler?

I'm pretty sure this is an unintentional side-effect of recursive-edit,
and now that you've pointed it out we need to decide whether there is
(a) any reason this shouldn't continue to work, or (b) whether it ought
to be necessary, i.e., why the handlers can't always have ZLE context.

} - Sometimes I have to deactivate the autosuggestions feature and
} consequently some of the widget hooks(eg: editing the middle of the line or
} in vi normal mode)

If deactivation is accomplished by changing a key binding, then this will
"just happen" for e.g. vi mode when the vicmd keymap replaces the one
used by recursive-edit.  If it's deactivated another way, keymap+widget
won't help, but won't hurt either.

} With the current issues in mind I have a few more questions:
} 
} - Is it possible to update the zle RBUFFER from outside a widget?

Only with "print -z" which still requires "zle get-line" at some point
later.

} ... is there a way to enter zle's context from a hook set with 'zle -F' ?

I don't think so, but PWS may know better.

} - Can the keymap+widget feature be used outside recursive-edit?

Yes, as long as you created the keymap with "bindkey -N newkeymap main"
after initializing keymap+widget, switching to that keymap will invoke
the widgets whose names are prefixed with the keymap name.  (It won't
work for other keymaps like vicmd without extra effort, nor for any of
the "special" keymaps like menuselect.)

recursive-edit is just a convenient way to semi-automaticall push/pop
keymaps.

} ... if thats the case I can simply switch
} keymaps with 'zle -K [keymap]' whenever I need a set of widget hooks to be
} active right?

Yes.

} - Whats the best way to modify RBUFFER before a completion widget is
} executed? I've tried the comppostfuncs but the RBUFFER variable is
} read-only in that context.

(comppostfuncs would modify the buffer after the completion widget was
executed.)

It might depend on what you're doing to RBUFFER but the only general
way I can think of is to start off with a normal widget and then call
the desired widget from inside it with "zle completion-widget-name".
If you start out in a widget defined with "zle -C" all of the normal
widget variables are already read-only and you can only affect RBUFFER
by changing what gets passed to compadd.

Re: Fish-like autosuggestions

[Peter Stephenson]

4On Tue, 05 Nov 2013 07:57:00 -0800
Bart Schaefer <schaefer@brasslantern.com> wrote:
> } - I'm only using recusive-edit because I couldn't do asynchronous updates
> } to the zle RBUFFER using 'zle -F'(I started from the 'predict-on' code)
> 
> Hmm; so the trick here is that if you start recursive-edit, then when
> the zle -F handler executes the special parameters from the surrounding
> (original) ZLE context are still defined in the handler?
> 
> I'm pretty sure this is an unintentional side-effect of recursive-edit,
> and now that you've pointed it out we need to decide whether there is
> (a) any reason this shouldn't continue to work, or (b) whether it ought
> to be necessary, i.e., why the handlers can't always have ZLE context.

I don't remember any good reason why zle -F hooks shouldn't be treated
as ZLE hooks rather than just function hooks.  I know the context I had
in mind when I implemented it was outside zle --- I was expecting the
function to invalidate the display and do something outside ZLE in the
way a job notification would --- but that simply says I wasn't thinking
about ZLE context rather than giving any reason.

It would seem natural to replace the callhookfunc() in raw_getbyte()
with a zlecallhook().  However, that's incompatible so it probably needs
augmenting with an additional option to zle -F to indicate the argument
is a ZLE widget rather than a shell function (a ZLE widget is a
different type of object and it would be silly to have two separate
mechanisms for ZLE widget hooks).  That may throw up some new problems
(not sure if terminal state is sane enough to permit editing the line
that deep inside the code for reading characters, for example), but they
ought to be fixable --- particularly if the recursive-edit hack is
basically working.

pws

Re: Fish-like autosuggestions

[Bart Schaefer]

On Nov 5,  4:18pm, Peter Stephenson wrote:
}
} It would seem natural to replace the callhookfunc() in raw_getbyte()
} with a zlecallhook().  However, that's incompatible so it probably needs
} augmenting with an additional option to zle -F to indicate the argument
} is a ZLE widget rather than a shell function

How about we treat it like zle-line-init, zle-keymap-select, etc.?  Have
a single predefined widget name that is called at that point (whether
before, after, or instead of the "zle -F" handlers, I don't really have
an opinion yet).  Pass the file descriptor number in $NUMERIC, and maybe
even set $PENDING to say how many bytes are ready on that FD.

Hmm, the doc doesn't actually explain what the return value from a -F
handler means to the surrounding code.  There should probably be some
sort of return-code-based protocol to indicate whether handling should
proceed, which makes me lean away from the "instead" option but still
doesn't resolve before/after for me.

Re: Fish-like autosuggestions

[Bart Schaefer]

On Nov 5, 11:46am, Bart Schaefer wrote:
}
} ... a single predefined widget name that is called at that point ...
} 
} Hmm, the doc doesn't actually explain what the return value from a -F
} handler means to the surrounding code.  There should probably be some
} sort of return-code-based protocol to indicate whether handling should
} proceed, which makes me lean away from the "instead" option but still
} doesn't resolve before/after for me.

A bit more examination leads me to feel that running the zle -F handler
first, and then skipping the widget ("zle-tcp-handler" ?) if the -F
function returns nonzero, is probably the most sensible way to go, both
for backward compatibility and because it's harder to interpret the
"failure" of a widget.

Re: Fish-like autosuggestions

[Peter Stephenson]

On Tue, 05 Nov 2013 12:40:00 -0800
Bart Schaefer <schaefer@brasslantern.com> wrote:
> On Nov 5, 11:46am, Bart Schaefer wrote:
> }
> } ... a single predefined widget name that is called at that point ...
> } 
> } Hmm, the doc doesn't actually explain what the return value from a -F
> } handler means to the surrounding code.  There should probably be some
> } sort of return-code-based protocol to indicate whether handling should
> } proceed, which makes me lean away from the "instead" option but still
> } doesn't resolve before/after for me.

It doesn't mean anything: it doesn't make sense for a function
listening for one file descriptor to cause aborting of a function or
widget associated with a different descriptor.  I'd suggest simply we
have either a widget or a normal function for each descriptor and the
problem goes away.  It doesn't make sense to process input from the file
descriptor twice so assigning two processor entities should just be an
error.

> A bit more examination leads me to feel that running the zle -F handler
> first, and then skipping the widget ("zle-tcp-handler" ?) if the -F
> function returns nonzero, is probably the most sensible way to go, both
> for backward compatibility and because it's harder to interpret the
> "failure" of a widget.

You'll have to associate a widget with a file descriptor, so it'll need
to have a specific name --- we could use magic names like
zle-tcp-handler-<fd>, but that's a bit messy and doesn't make clear the
competition between reading the fd this way and the original way.  We
can't just use a generic widget because we don't know what file
descriptors we're listening on.  Listing file descriptors separately
from defining a widget loses both functionality and coherence --- if you
have multiple file descriptors there's no reason to suppose you want to
call the same widget for each, indeed there very likely to be doing
completely different tasks.  So I think the easiest thing is just
slightly augment the interface with an option to say the argument names
a widget rather than a function.  This also makes the implementation
easy: we just do something a little bit different based on a flag
associated with each string in the list.

pws

Re: Fish-like autosuggestions

[Bart Schaefer]

On Wed, Nov 6, 2013 at 12:07 PM, Peter Stephenson
<p.w.stephenson@ntlworld.com> wrote:
> On Tue, 05 Nov 2013 12:40:00 -0800
> Bart Schaefer <schaefer@brasslantern.com> wrote:
>> On Nov 5, 11:46am, Bart Schaefer wrote:
>> }
>> } Hmm, the doc doesn't actually explain what the return value from a -F
>> } handler means to the surrounding code.
>
> It doesn't mean anything: it doesn't make sense for a function
> listening for one file descriptor to cause aborting of a function or
> widget associated with a different descriptor.

OK, I only asked because the examples seem to have different return
values for success/failure, and I wanted to be sure that wasn't more
than a programming habit.

> You'll have to associate a widget with a file descriptor, so it'll need
> to have a specific name [... and]
> can't just use a generic widget because we don't know what file
> descriptors we're listening on.

You've already implemented your idea so perhaps this is moot, but to
explain my thought: The same widget could get invoked for all
descriptors as long as it has a way to tell which descriptor caused
the call.  Hence my suggestion of putting the descriptor number in
$NUMERIC etc.; the single widget could just do "case $NUMERIC in ..."
or something (such as passing the FD as an argument, which appears to
be what you did).

Re: Fish-like autosuggestions

[Peter Stephenson]

On Wed, 06 Nov 2013 16:04:23 -0800
Bart Schaefer <schaefer@brasslantern.com> wrote:
> You've already implemented your idea so perhaps this is moot, but to
> explain my thought: The same widget could get invoked for all
> descriptors as long as it has a way to tell which descriptor caused
> the call.  Hence my suggestion of putting the descriptor number in
> $NUMERIC etc.; the single widget could just do "case $NUMERIC in ..."
> or something (such as passing the FD as an argument, which appears to
> be what you did).

I can see that --- and it's probably entirely moot at this point --- but
I think of it this way: you've set up a file descriptor for a particular
purpose.  Rather than edit an existing widget handling file descriptors
for completely different purposes, which is a maintainance problem, you
probably want to run a completely different chunk of code.  In other
words, what's needed is a bit more like binding a keystroke to a
widget than running a fixed widget in a specific circumstance.  The
semantics of zle -F have that binding effect.

Anyway, I think it's down to seeing if the change does the trick.  By
the way, note that using "zle -F" with an existing file descriptor
silently overwrites the previous handler (another parallel with key
binding), even if the mechanism has changed, which partially contradicts
what I said before.

I'll probalby commit this before 5.0.3 (not 4.0.3) since it shouldn't
break anything existing and I should be able to confirm that as I use
the old interface all the time.

pws

Re: Fish-like autosuggestions

[Thiago Padilha]

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

On Thu, Nov 7, 2013 at 6:44 AM, Peter Stephenson
<p.stephenson@samsung.com>wrote:

>
> I'll probalby commit this before 5.0.3 (not 4.0.3) since it shouldn't
> break anything existing and I should be able to confirm that as I use
> the old interface all the time.
>
> pws
>


Does that mean in zsh 5.0.3 'zle -F' hooks will enter zle context
automatically?

If so I'm going to use the 'is-at-least' function as a condition to enable
the completion server and only work with history suggestions for lower
versions

Re: Fish-like autosuggestions

[Peter Stephenson]

On Thu, 07 Nov 2013 16:07:28 -0200
Thiago Padilha <tpadilha84@gmail.com> wrote:
 On Thu, Nov 7, 2013 at 6:44 AM, Peter Stephenson
> <p.stephenson@samsung.com>wrote:
> > I'll probalby commit this before 5.0.3 (not 4.0.3) since it shouldn't
> > break anything existing and I should be able to confirm that as I use
> > the old interface all the time.
>
> Does that mean in zsh 5.0.3 'zle -F' hooks will enter zle context
> automatically?

No, for the new feature you need an additional flag (zle -Fw).  It's
completely backward compatible (I've been using it in the old mode
during the day).

pws