Zsh-autosuggestions: Using vi-forward-char to accept widgets triggers bell

[fgr0]

Describe the bug

Using vi-forward-char in _either_ viins or vicmd to accept a suggestion will trigger a bell after successfully entering the suggestion.

To Reproduce

Steps to reproduce the behavior:

```.zsh
% zsh -df
% source ~/.../zsh-autosuggestions.zsh
% bindkey -v
% source ... # use right-arrow/vi-forward-char, bell is triggered
% bindkey -e
% source ... # use right-arrow/forward-char, bell is not triggered

### Expected behavior
After successfully accepting a suggestion the bell should not be triggered.

### Desktop
 - OS + distribution: macOS Catalina 10.15.1
 - Zsh version: 5.7.1
 - Plugin version: current master (d43c309f) and develop (bdbe43e6)

### Additional context
I believe that this is due to an indirect/maybe off-by-one-error inside the `_zsh_autosuggest_accept()` widget when calculating the new cursor position. Currently, the cursor-position is calculated using https://github.com/zsh-users/zsh-autosuggestions/blob/d43c309f888153d6c46d8b6a3a0186f4148680fd/src/widgets.zsh#L142
which will move the cursor to the end of the buffer (right after the last character). After this the original widget is called, which will try move the cursor _again_. In the case `vi-forward-char` this move will fail, since you cannot move beyond the end of the buffer, and trigger the bell. The normal `forward-char` on the other hand will also try to move past the end of the buffer (and fail), but it will not trigger a bell.

The difference in behavior between `forward-char` and `vi-forward-char` can also be observed without zsh-autosuggestion:

```.zsh
% zsh -df
% bindkey -v
%                          # empty line, use right-arrow/vi-forward-char, triggers bell
% bindkey -e
%                          # empty line, use right-arrow/forward-char, is quiet

This does not appear to be a recent change in zsh and I think that this is expected behavior.

In an effort to prevent the constant carol of the bells in my terminal I have changed the relevant part of the _zsh_autosuggest_accept() function to move the cursor _onto the last character_ instead of _after_ it by reducing CURSOR by 1. This way, the call to the original widget can still move the cursor as it should and will not fail:

```.zsh
diff --git a/src/widgets.zsh b/src/widgets.zsh
index 242d502..9ee5f67 100644
--- a/src/widgets.zsh
+++ b/src/widgets.zsh
@@ -128,18 +128,19 @@ _zsh_autosuggest_accept() {
fi

# Only accept if the cursor is at the end of the buffer

- if [[ $CURSOR = $max_cursor_pos ]]; then
+ if [[ $CURSOR = $max_cursor_pos && ! -z $POSTDISPLAY ]]; then
# Add the suggestion to the buffer
BUFFER="$BUFFER$POSTDISPLAY"

    # Remove the suggestion
    unset POSTDISPLAY

• # Move the cursor to the end of the buffer
• # Move the cursor to the end of the buffer - 1, because the original
• # widget is going to move it once more
  if [[ "$KEYMAP" = "vicmd" ]]; then
• CURSOR=$(($#BUFFER - 1))
• CURSOR=$(($#BUFFER - 2))
  else
• CURSOR=$#BUFFER
• CURSOR=$(($#BUFFER - 1))
  fi
  fi

```

In my testing, this change does not change the behavior of any of the default $ZSH_AUTOSUGGEST_ACCEPT_WIDGETS widgets and works fine with both bindkey -e and bindkey -v; no matter the keymap, the cursor will appear in exactly the same position as before my patch.

The reason I did not create a PR right away is that my version will break behavior for any widgets that do not move the cursor at least one char on their own. This also means, that the rspec ./spec/options/widget_lists_spec.rb:8 will fail when using the patch, since it uses an empty function as a widget. I am not sure what to do about this; one approach may be to just use $#BUFFER - 1 for vi-forward-char but that would require hardcoding that widget-name.

If you want, I can upload my changes and create a PR anyway while we try to find a way around this problem. I hope I didn't miss anything, thanks for your help!

[fgr0]

After seeing the diff at https://github.com/zsh-users/zsh-autosuggestions/pull/294#issuecomment-425930930 I think that calling _zsh_autosuggest_invoke_original_widget $@ before setting the cursor is indeed the right way to go! It passes all the tests and fixes the superfluous beep when accepting. So this might actually be a way to fix both problems at the same time?

Here is a current patch based directly on @frebib's fix.

```.patch
diff --git a/src/widgets.zsh b/src/widgets.zsh
index 242d502..c2046cb 100644
--- a/src/widgets.zsh
+++ b/src/widgets.zsh
@@ -128,22 +128,24 @@ _zsh_autosuggest_accept() {
fi

# Only accept if the cursor is at the end of the buffer

- if [[ $CURSOR = $max_cursor_pos ]]; then
+ if [[ $CURSOR = $max_cursor_pos && ! -z "$POSTDISPLAY" ]]; then
# Add the suggestion to the buffer
BUFFER="$BUFFER$POSTDISPLAY"

    # Remove the suggestion
    unset POSTDISPLAY

• _zsh_autosuggest_invoke_original_widget $@
  +
  # Move the cursor to the end of the buffer
  if [[ "$KEYMAP" = "vicmd" ]]; then
  CURSOR=$(($#BUFFER - 1))
  else
  CURSOR=$#BUFFER
  fi
• else
• _zsh_autosuggest_invoke_original_widget $@
  fi
  -

• _zsh_autosuggest_invoke_original_widget $@
  }

  Accept the entire suggestion and execute it

  ```

[ericfreese]

Thanks @fgr0! Please try out branch fixes/run-orig-before-move-cursor (PR #488) and let me know if it solves the issue for you.

[fgr0]

Tried out #488 and it fixes the issue for me, thanks!