kutsan / zsh-system-clipboard

System clipboard key bindings for Zsh Line Editor with vi mode. It is similar to what `set clipboard=unnamed` does for vim.
GNU General Public License v3.0
140 stars 16 forks source link

Command not found: _zsh_system_clipboard_set #25

Closed laggardkernel closed 5 years ago

laggardkernel commented 5 years ago

Press x in normal mode.

zsh-system-clipboard-set:2: command not found: _zsh_system_clipboard_set
kutsan commented 5 years ago

https://github.com/kutsan/zsh-system-clipboard/blob/447735132be3e6d4d8d7e8f2757d9c1d3504124b/zsh-system-clipboard.zsh#L113

At this line of the plugin file, could you please add eval to the beginning of that line? Like this:

- _zsh_system_clipboard_set
+ eval _zsh_system_clipboard_set

It's to check if it's issue with aliases. Also, can you run setopt | grep alias in terminal, what's the output?

laggardkernel commented 5 years ago

After adding eval before the line, nothing is changed, the same err is thrown out. Besides, no output is got from setopt | grep alias.

I can confirm it's an alias problem. I'm using zsh 5.7.1 on macOS. The problem could be fixed with converting the aliases to functions.

-           alias _zsh_system_clipboard_set='pbcopy'
-           alias _zsh_system_clipboard_get='pbpaste'
+           function _zsh_system_clipboard_set { pbcopy; }
+           function _zsh_system_clipboard_get { pbpaste; }
doronbehar commented 5 years ago

My setopt | grep alias is empty as well. @laggardkernel do you source zsh-system-clipboard.zsh in your ~/.zshrc or do you use a plugin manager such as zplugin?

laggardkernel commented 5 years ago

@doronbehar I'm using zplugin. But it has nothing to do with the plugin manager. There's no difference between how the plugin is loaded, by a plugin manager or being sourced manually. Cause the only way to load a plugin is to source the related .zsh files.

The problem is caused by the method used to wrap different clipboard executables, alias. The alias _zsh_system_clipboard_set is not expanded into the real executable unless setopt ALIAS is enabled.

To put it simple, alias is not a reliable way to wrap commands and use them later. function or array is preferred.

function _zsh_system_clipboard_set { pbcopy; }
# use it as, _zsh_system_clipboard_set

# or
typeset -ga _zsh_system_clipboard_set=(pbcopy)
# use it as, ${_zsh_system_clipboard_set[@]}
doronbehar commented 5 years ago

@laggardkernel have you tested this by manually sourcing zsh-system-clipboard instead of using it with zplugin? My suspicion is that zplugin somehow sources plugins in an isolated environment and aliases are not supported there. We've also had a perhaps similar issue with hash which didn't recognize the commands xsel or xclip while type did - #24, not sure if it's related.

I will tell you though, that the reason we are using aliases instead of functions is because of this part. https://github.com/kutsan/zsh-system-clipboard/blob/447735132be3e6d4d8d7e8f2757d9c1d3504124b/zsh-system-clipboard.zsh#L44-L85

The 'problem' is, that I would like to avoid having a function that will interpret a global environmental variable which it's value could be -sel CLIPBOARD for xclip or -b for xsel. This variable would have had to be dynamically evaluated in these functions, with eval as well. In contrast, defining a 'static' alias on startup according to the requested clipboard selection ZSH_SYSTEM_CLIPBOARD_SELECTION seems the most elegant way to avoid that, but I'm open to new ideas. Additionally, it'd be worth mentioning that In the past, before working on f5b32d5, I've had the suspicion that an eval inside this function may effect the actual contents that reach the clipboard.

Since our startup code already looks like spaghetti, I tend to think maybe we shouldn't consider so many edge cases and print such nice messages in case we fail every attempt to detect a clipboard command. Perhaps even official support for tmux buffers should be dropped and users should define their own aliases if they wish to change the default behavior meaning this should be taken care of in a matter of documentation.. Not sure, just thinking out loud @kutsan.

laggardkernel commented 5 years ago

@doronbehar You're right, the err is indeed related with zplugin. sourceing the plugin directly makes the plugin work as expected.

I tried to define _zsh_system_clipboard_get and _zsh_system_clipboard_set as arrays to avoid the problem, and it works. (At least for me, with and without tmux on macOS). Detail in GH-26

Talking about determining whether a command exist, if (( $+commands[xsel] )); then seems to be the zsh way doing this. whether-a-command-exists.md

Update: Enabling alias expansion locally seems to be more simple. Another pr is added. Pick the one you like.

doronbehar commented 5 years ago

@laggardkernel thanks again for your fix. Would you like to send another PR with if (( $+commands[xsel] )); then used instead of type?