Add an autosuggest widget: autosuggest-execute. #122

Closed
wants to merge 1 commit into
from

2 participants

@hitripod

Add back autosuggest-execute-suggestion from #61 which is removed after rewriting.

@ericfreese ericfreese commented on an outdated diff Feb 21, 2016
@@ -81,6 +82,7 @@ Widgets not in any of these lists will update the suggestion when invoked.
This plugin provides two widgets that you can use with `bindkey`:
1. `autosuggest-accept`: Accepts the current suggestion.
+2. `autosuggest-execute`: Accepts the current suggestion and execute it.
@ericfreese
zsh-users member
ericfreese added a line comment Feb 21, 2016

Let's reword this. How about "Accepts and executes the current suggestion."?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@ericfreese ericfreese commented on the diff Feb 21, 2016
src/config.zsh
@@ -30,6 +30,14 @@ ZSH_AUTOSUGGEST_ACCEPT_WIDGETS=(
vi-end-of-line
)
+# Widgets that accept the entire suggestion and execute it
+ZSH_AUTOSUGGEST_EXECUTE_WIDGETS=(
@ericfreese
zsh-users member
ericfreese added a line comment Feb 21, 2016

I think this array should be empty by default. All of these widgets already exist in the ACCEPT array, and shouldn't be in both. Users who want these widgets to execute instead of accept could then remove these widgets from the ACCEPT array and add to the EXECUTE array in their own configuration after sourcing the plugin

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@ericfreese ericfreese commented on an outdated diff Feb 21, 2016
src/widgets.zsh
@@ -47,6 +47,23 @@ _zsh_autosuggest_accept() {
_zsh_autosuggest_invoke_original_widget $@
}
+# Accept the entire suggestion and execute it
+_zsh_autosuggest_execute() {
+ # Only accept if the cursor is at the end of the buffer
+ if [ $CURSOR -eq $#BUFFER ]; then
@ericfreese
zsh-users member
ericfreese added a line comment Feb 21, 2016

It looks like the original implementation didn't have this check to see if the cursor is at the end of the line. I think we should go ahead and remove this if statement and execute the suggestion regardless of where the cursor is.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@ericfreese ericfreese commented on an outdated diff Feb 21, 2016
src/widgets.zsh
@@ -47,6 +47,23 @@ _zsh_autosuggest_accept() {
_zsh_autosuggest_invoke_original_widget $@
}
+# Accept the entire suggestion and execute it
+_zsh_autosuggest_execute() {
+ # Only accept if the cursor is at the end of the buffer
+ if [ $CURSOR -eq $#BUFFER ]; then
+ # Add the suggestion to the buffer
+ BUFFER="$BUFFER$POSTDISPLAY"
+
+ # Remove the suggestion
+ unset POSTDISPLAY
+
+ # Move the cursor to the end of the buffer
+ CURSOR=${#BUFFER}
@ericfreese
zsh-users member
ericfreese added a line comment Feb 21, 2016

Moving the cursor shouldn't be necessary when executing the suggestion

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@ericfreese ericfreese commented on an outdated diff Feb 21, 2016
src/widgets.zsh
@@ -47,6 +47,23 @@ _zsh_autosuggest_accept() {
_zsh_autosuggest_invoke_original_widget $@
}
+# Accept the entire suggestion and execute it
+_zsh_autosuggest_execute() {
+ # Only accept if the cursor is at the end of the buffer
+ if [ $CURSOR -eq $#BUFFER ]; then
+ # Add the suggestion to the buffer
+ BUFFER="$BUFFER$POSTDISPLAY"
+
+ # Remove the suggestion
+ unset POSTDISPLAY
+
+ # Move the cursor to the end of the buffer
+ CURSOR=${#BUFFER}
+ fi
+ _zsh_autosuggest_invoke_original_widget $@
+ _zsh_autosuggest_invoke_original_widget accept-line
@ericfreese
zsh-users member
ericfreese added a line comment Feb 21, 2016

I think we should be able to just go ahead and use zle .accept-line here instead

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@hitripod

@ericfreese Thanks for your nice review! Revised.

@ericfreese ericfreese commented on the diff Feb 22, 2016
src/widgets.zsh
@@ -47,6 +47,11 @@ _zsh_autosuggest_accept() {
_zsh_autosuggest_invoke_original_widget $@
}
+# Accept the entire suggestion and execute it
+_zsh_autosuggest_execute() {
@ericfreese
zsh-users member
ericfreese added a line comment Feb 22, 2016

I believe we still need to set the buffer and clear the postdisplay here before accepting the line.

@hitripod
hitripod added a line comment Feb 22, 2016

Yes, you're right. It's my mistake.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@ericfreese ericfreese commented on an outdated diff Feb 22, 2016
@@ -81,7 +82,8 @@ Widgets not in any of these lists will update the suggestion when invoked.
This plugin provides two widgets that you can use with `bindkey`:
@ericfreese
zsh-users member
ericfreese added a line comment Feb 22, 2016

Can you update this line to say "...provides three widgets..."?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@ericfreese
zsh-users member

Can you update your PR to merge into the features/execute_widget branch instead of master?

@hitripod

Close to retarget the branch. See PR #124.

@hitripod hitripod closed this Feb 22, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment