A possibly needed workaround for <( ) process substitution

[psprint]

Hello,
in branch develop there's currently use of <( ... ) process substitution to do asynchronous calls:

zsh-autosuggestions/zsh-autosuggestions.zsh

Lines 717 to 725 in 681ffc7

	exec {_ZSH_AUTOSUGGEST_ASYNC_FD}< <(
	# Tell parent process our pid
	echo $sysparams[pid]
	# Fetch and print the suggestion
	local suggestion
	_zsh_autosuggest_fetch_suggestion "$1"
	echo -nE "$suggestion"
	)

I use the same technique in fast-syntax-highlighting, however, I add command sleep 0 (could make it command true) after the exec: https://github.com/zdharma/fast-syntax-highlighting/blob/e06288a617b12e8038fdf5bc4bd40b6a25ef5332/fast-highlight#L917-L918

The reason for this additional call are severe problems with Ctrl-C that occur when using {MYFD}< construct, as my investigation showed.

So in this issue I just want to mention that in case of any Ctrl-C problems, command true after exec should help.

[ericfreese]

Thanks for this! I was actually having issues with Ctrl-C on develop but wasn't sure exactly why and hadn't yet put in the effort to figure out what was going on. I'm busy for the next few weeks but I'll take a closer look at this before cutting a release from develop.

[ericfreese]

@psprint Did you ever figure out why exactly this was necessary? I'm really curious what's going on that makes ctrl-c stop working.

[ericfreese]

I've found something interesting that you may have also found. At least in my case, it seems the problem only occurs with the monitor option set (does not occur with it unset).

[psprint]

Interesting. I was tracking this for many days and when workaround appeared I was too tired to continue, so I dont know why this problem occurs

[ericfreese]

Fair enough. I'll see what I can find...

[ericfreese]

@psprint I've submitted a patch that fixes this issue upstream

http://www.zsh.org/mla/workers/2019/msg00252.html

[ericfreese]

FYI @psprint it looks like this workaround doesn't work for zsh versions older than 5.0.8.

[ericfreese]

Closing this. I've added the workaround with PR #417

[psprint]

Great job! Wt., 9.04.2019, 20:36 użytkownik Eric Freese <notifications@github.com> napisał:

…

@psprint <https://github.com/psprint> I've submitted a patch that fixes this issue upstream http://www.zsh.org/mla/workers/2019/msg00252.html — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#364 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AFxOCIlUus63CaFRz0WMWCxdWyLv9AjFks5vfN2bgaJpZM4VyxCm> .

[abcdw]

Can we use command -v true instead of command true here?

command true breaks, when I have no true binary in my $PATH, it is relatively often use case for me, because I have pure environment for various build tasks using guix environment --pure.

BTW, Thank you very much for your great work.

[danielshahaf]

Can we use command -v true instead of command true here?

Probably not, given that the comment just above that line of code says a fork is needed, and command -v true doesn't fork. (Also, it spams stdout, so a > /dev/null would be required.)

zsh-autosuggestions/src/async.zsh

Lines 45 to 47 in ae315de

	# There's a weird bug here where ^C stops working unless we force a fork
	# See https://github.com/zsh-users/zsh-autosuggestions/issues/364
	command true

command true breaks, when I have no true binary in my $PATH, it is relatively often use case for me, because I have pure environment for various build tasks using guix environment --pure.

Your setup violates POSIX (https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap01.html#tag_17_06) and therefore is likely to break other upstreams too:

However, all of the standard utilities, including [true] shall be implemented in a manner so that they can be accessed via the exec family of functions [...]

(I'm not a z-asug developer.)

[abcdw]

@danielshahaf Thank you for the reply!

However, all of the standard utilities, including [true] shall be implemented in a manner so that they can be accessed via the exec family of functions [...]

Not sure if it violates, exec true works perfectly fine just out of the box.

>> which true
true: shell built-in command

>> command true
zsh: command not found: true

>> setopt posixbuiltins
>> command true
>>

Probably not, given that the comment just above that line of code says a fork is needed.

I'm not sure how this workaround works, but maybe something like $(true) will work too?

I get a lot of _zsh_autosuggest_async_request:41: command not found: true when I'm in pure environment without true binary. It's quite annoying. For now I disabled async for pure shells with [ -n $GUIX_ENVIRONMENT ] || ZSH_AUTOSUGGEST_USE_ASYNC=true, but looks a little hacky. Would be glad to have more robust solution, but I'm not very into shells development and do not know all the tricky things related to it(

[ericfreese]

There is a test for this workaround here

zsh-autosuggestions/spec/async_spec.rb

Line 53 in ae315de

describe 'pressing ^C after fetching a suggestion' do

If you can get the tests running, you can try some alternatives to command true to see if we can work around the issue in a different way.

The upstream bug has also been fixed in more recent versions of zsh so at this point we may be able to only run the command true for certain older versions. I know that the workaround doesn’t work for versions older than 5.0.8 but I would have to look into the exact version that the bug was fixed in.

[danielshahaf]

Not sure if it violates, exec true works perfectly fine just out of the box.

The quoted sentence refers to the execvp(3) C function, not to the exec utility/builtin.

Incidentally, zsh's exec can only be used to execute the true builtin when POSIX_BUILTINS is unset; see the third paragraph of POSIX_BUILTINS' documentation.

Would be glad to have more robust solution, but I'm not very into shells development and do not know all the tricky things related to it(

I recommend to install in the build environment whichever package provides a true executable in $PATH.

The upstream bug has also been fixed in more recent versions of zsh so at this point we may be able to only run the command true for certain older versions. I know that the workaround doesn’t work for versions older than 5.0.8 but I would have to look into the exact version that the bug was fixed in.

https://zsh.org/workers/44214 is zsh commit zsh-5.7.1-46-g5c165453a, first released in 5.8.

[abcdw]

--- Sorry for the long response, I've set up a testing environment, made a fix and checked it on versions 5.6.2 and 5.8. Tests are passed. src/async.zsh | 3 ++- zsh-autosuggestions.zsh | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/async.zsh b/src/async.zsh index 4314e8c..218eb26 100644 --- a/src/async.zsh +++ b/src/async.zsh @@ -44,7 +44,8 @@ _zsh_autosuggest_async_request() { # There's a weird bug here where ^C stops working unless we force a fork # See #364 - command true + autoload -Uz is-at-least + is-at-least 5.8 || command true # Read the pid from the child process read _ZSH_AUTOSUGGEST_CHILD_PID <&$_ZSH_AUTOSUGGEST_ASYNC_FD diff --git a/zsh-autosuggestions.zsh b/zsh-autosuggestions.zsh index a8ef6c4..7ebc490 100644 --- a/zsh-autosuggestions.zsh +++ b/zsh-autosuggestions.zsh @@ -804,7 +804,8 @@ _zsh_autosuggest_async_request() { # There's a weird bug here where ^C stops working unless we force a fork # See #364 - command true + autoload -Uz is-at-least + is-at-least 5.8 || command true # Read the pid from the child process read _ZSH_AUTOSUGGEST_CHILD_PID <&$_ZSH_AUTOSUGGEST_ASYNC_FD -- 2.31.1

[abcdw]

Dear maintainers, is it possible to apply this patch? Seems like a reasonable solution to me.

[ericfreese]

Hi, thanks for the patch! Yeah, we should be able to apply this. I'm sorry I haven't had much time for this project lately but should be able to get this in soon. I've also been meaning to cut a new release for some time.

Can you send a pull request to develop branch with these changes?

[abcdw]

Sure!