Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP : When using vi-mode and zsh-syntax-highlighting, command is not highlighted after accepting suggestion #294

Closed
wants to merge 12 commits into from

Conversation

toadjaune
Copy link

I accidentally closed #260 by rebasing on the top of develop, so I'm reopening it here.

I'll be trying to fix it properly soon.

@ericfreese
Copy link
Member

I really don't want to have code here referencing another plugin. That's a slippery slope that will lead to this plugin getting bloated with all kinds of crazy stuff to support other plugins. Lets find another way to fix this if possible.

Also, it seems like this bug could be described better as "When using vi-mode and zsh-syntax-highlighting, command is not highlighted after accepting suggestion", since the plugin is already largely compatible with z-sy-h. #6 is a very old issue, and is, in fact, already closed.

@toadjaune
Copy link
Author

I don't want to have code referencing another plugin as well, that's why I'm trying to setup something cleaner. I definitely don't consider this hack to be mergeable as-is, I'm adding a WIP tag in the title right now, to make it clear.

I think we should be able to remove these references by replacing the manual cursor movement by a call to a widget such as end-of-line. The idea is that plugins such as zsh-syntax-highlighting wrap every existing zle widget, so by using such a call, we should be able to have the syntax highlighting updated (or any other crazy thing done by another plugin) just the way it would be if the user had typed it himself.

I'm working on it, but right now I have a recursive function call causing a stack overflow.
Anyway, I'll either push something working when I starts working, or ask for help if I can't figure it out.

@toadjaune toadjaune changed the title Compatibility with zsh-syntax-highlighting WIP : When using vi-mode and zsh-syntax-highlighting, command is not highlighted after accepting suggestion Nov 27, 2017
@toadjaune
Copy link
Author

toadjaune commented Dec 4, 2017

Here is a cleaner version, we're getting closer to something acceptable.

I still need to figure out how the _zsh_autosuggest_bind_widget function works, especially the $bind_count variable. It's gonna be required to prevent hardcoding the autosuggest-orig-1-end-of-line widget.
So far, I have no idea in which case it may go above 1.

We also still need to care about the _zsh_autosuggest_execute function.

I tried to check whether the tests pass or not, but I can't get them to pass even on master, so… well, if someone can give it a try, I'd be really grateful !

@toadjaune
Copy link
Author

toadjaune commented Dec 4, 2017

Oh, by the way, I couldn't install ruby 2.3.1 because of this issue, that's why I bumped the ruby version up. Since the tests don't work at all for me, I can revert this change if it is the source of the problem.

@ericfreese
Copy link
Member

@toadjaune Is this still an issue? I'm not able to reproduce with following versions:

  • zsh @ 5.4.2
  • z-asug @ b2b9bf3
  • z-syn-hi @ 5b53966

@toadjaune
Copy link
Author

Yes it is, I was working on it when I ran into #324. I'll try and have a look in the weeks to come.

@toadjaune
Copy link
Author

I just tested, and the bug is still present for me, with the following versions :

  • zsh: 5.4.2
  • zsh-autosuggestions: current develop (ie 5549b68, newer than b2b9bf3)
  • zsh-syntax-highlighting: current master (ie ee26d44, newer than 5b53966)

I'd be glad to write a test showing the erroneous behaviour, but I'm not sure how to do this without loading zsh-syntax-highlighting, which would basically be an integration test (with all the version synchronization issues that arise from these).
Do you want more detailed reproduction instructions ?

In the meantime, I've merged develop into my branch, and tested again.
My fix still works fine, and does not break any test. (I do however have 3 tests that were not passing in the first place on current develop)

@toadjaune toadjaune force-pushed the compatibility-syntax-hl branch from e8bed2f to 5541533 Compare May 31, 2018 16:33
@ericfreese
Copy link
Member

I've finally reproduced this! I have ^E bound to end-of-line in vi mode, and that's what I usually use to accept the suggestion. However, when I use the right arrow key to trigger vi-forward-char to accept the suggestion, I do see this behavior. I'm not sure how they have different behavior, but at least now I can reproduce. Will look into this more soon.

@frebib
Copy link

frebib commented Oct 1, 2018

It seems to be caused by updating the CURSOR before calling _zsh_autosuggest_invoke_original_widget. I'm starting to think this is also partially caused by zsh-synhl too, perhaps an optimisation or oversight in that?

Here is an alternative fix that also solves the issue:

diff --git a/zsh-autosuggestions.zsh b/zsh-autosuggestions.zsh
index 1c3eab5..d94fa42 100644
--- a/zsh-autosuggestions.zsh
+++ b/zsh-autosuggestions.zsh
@@ -412,11 +412,13 @@ _zsh_autosuggest_accept() {
 		# Remove the suggestion
 		unset POSTDISPLAY
 
+		_zsh_autosuggest_invoke_original_widget $@
+
 		# Move the cursor to the end of the buffer
 		CURSOR=${#BUFFER}
+	else
+		_zsh_autosuggest_invoke_original_widget $@
 	fi
-
-	_zsh_autosuggest_invoke_original_widget $@
 }
 
 # Accept the entire suggestion and execute it

@toadjaune
Copy link
Author

This should have been closed a long time ago since #488 fixed it :)

@toadjaune toadjaune closed this Dec 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants