hlissner / zsh-autopair

Auto-close and delete matching delimiters in zsh
MIT License
509 stars 37 forks source link

Pairs should be expanded with an additional space right after the cursor #13

Closed kutsan closed 6 years ago

kutsan commented 6 years ago

See the GIF. This also applies for other pairs.

hlissner commented 6 years ago

Modifying the spacebar can be tricky, it won't work if you bind something to it after autopair is loaded, but in case you haven't, I've added the behavior you want.

Give it a try and let me know!

kutsan commented 6 years ago

Wow, thanks for quick response. It works flawlessly. But as you said, my other widgets are not working anymore. Isn't really there any workaround for this? Do I have to choose one of them?

kutsan commented 6 years ago

zsh-autosuggestions is wrapping user defined zle widgets, so they won't be obsolete.

hlissner commented 6 years ago

By other widgets, do you only mean zsh-autosuggestions, or are there other plugins zsh-autopair is conflicting with? Also, without zsh-autopair, could you tell me what you have space bound to (with bindkey " ")?

kutsan commented 6 years ago

I have this, bound to the space key in viins keymap mode: https://github.com/kutsan/dotfiles/blob/4f84241c1a27e9f4b730e1c0846e562832665bb3/.zsh/.zshrc#L590-L597

I sourced zsh-autopair after those line and after zsh-autopair I sourced zsh-autosuggestions. With your new commit bce78cd my widget is still not working. Although, zsh-autosuggestions works exactly as intended and I have this problem with zsh-autopair:

hlissner commented 6 years ago

Just pushed another adjustment which should fix your problem.

kutsan commented 6 years ago

This time works but I have two new problems.

I have setopt WARN_CREATE_GLOBAL in my configs and I pressed Backspace just before the error.

There shouldn't be any additional space right after the 2.

I think additional space should only be added if there is no text between pairs.

hlissner commented 6 years ago

I've pushed fixes for both problems. Let me know if you stumble across any other issues!

kutsan commented 6 years ago

Thanks again for everything. I couldn't find any problem this time. The only problem left is user-defined widget for space key is not working. What's your thoughts about that?

z0rc commented 6 years ago

Hey, commit 05e77dca4b6501752f99c902a182b83375b8ae07 breaks zsh subshell in midnight commander. I'm not sure what's going on there, but seems that for some reason zsh launched within mc fails to return working prompt.

z0rc commented 6 years ago

Just to confirm, ff7224244c0642ec68a0b59ae11bdaa8b72b7142 works fine under mc.

kutsan commented 6 years ago

@z0rc What's your version? Tried your problem but couldn't trigger it. It works fine for me. My MC version was 4.8.19, and tried under tmux 2.6 and zsh 5.4.2.

z0rc commented 6 years ago

I'm on Ubuntu 17.04 with all packages installed from official repositories. I'm not using tmux, but it doesn't matter, issue is reproducible with and without tmux. .zshrc containing single line:

source autopair.plugin.zsh

Versions installed:

% zsh --version            
zsh 5.2 (x86_64-ubuntu-linux-gnu)

% mc --version
GNU Midnight Commander 4.8.18
Built with GLib 2.50.1
Using the S-Lang library with terminfo database
With builtin Editor
With subshell support as default
With support for background operations
With mouse support on xterm and Linux console
With support for X11 events
With internationalization support
With multiple codepages support
Virtual File Systems: cpiofs, tarfs, sfs, extfs, ext2undelfs, ftpfs, sftpfs, fish
Data types: char: 8; int: 32; long: 64; void *: 64; size_t: 64; off_t: 64;

% tmux -V      
tmux 2.3
hlissner commented 6 years ago

@z0rc What zsh options do you have enabled? (run setopt to get a list)

z0rc commented 6 years ago

@hlissner

% setopt
interactive
monitor
shinstdin
zle
z0rc commented 6 years ago

If that matters, mc launches zsh as zsh -Z -g, which is:

ZLE (-Z)
Use the zsh line editor. Set by default in interactive shells connected to a terminal.

-g
HIST_IGNORE_SPACE
z0rc commented 6 years ago

I just confirmed, issue is reproducible with Ubuntu LTS 16.04 and zsh 5.1.1. I'm considering to drop this plugin from my zsh configuration as this feature breaks way more then helps.

hlissner commented 6 years ago

@kutsan I added a new envvar for specifying the fallback widget, try: AUTOPAIR_SPC_WIDGET=custom-expand-global-alias. Let me know how well that works for you.

hlissner commented 6 years ago

@z0rc I'm sorry to hear that. I couldn't reproduce it on Ubuntu 17 with zsh5.2 or 5.1.1, nor could I on Arch Linux with zsh 5.4+. I'm struggling to find the cause and I don't use midnight commander so I have no insight there.

I just pushed https://github.com/hlissner/zsh-autopair/commit/72ecea613516de7038cbb47355016b730567948f as a hail Mary, hoping that this is some esoteric quirk of quoting regex in zsh.

If it doesn't help, then I'll add an option to let you disable the functionality the new line in https://github.com/hlissner/zsh-autopair/commit/05e77dca4b6501752f99c902a182b83375b8ae07 is tied to. Please let me know!

hlissner commented 6 years ago

@zOrc oh, another question: what does bindkey " " | cut -c5- output for you? If this is failing, then that would explain your problem.

kutsan commented 6 years ago

@hlissner I found a typo here.

https://github.com/hlissner/zsh-autopair/blob/cd3bb9c0ea50342c4ee3d98719ef0b47bc6eef7c/autopair.zsh#L143

It should be SPC, not SRC. Fixed that locally and now it works. Thanks! I would create a PR for that but probably it's easier to tell your from here.

And also, I didn't write AUTOPAIR_SPC_WIDGET=custom-expand-global-alias to my .zshrc. Your AUTOPAIR_SPC_WIDGET=$(bindkey " " | cut -c5-) works okay.

You can close this issue as you wish. Thanks again!

hlissner commented 6 years ago

Whoops! Good catch. I've corrected it.

z0rc commented 6 years ago

@hlissner bindkey " " | cut -c5- in all cases returns autopair-insert.

Here is Dockerfile to quickly reproduce the issue:

FROM ubuntu
RUN apt-get update && apt-get -y install git mc zsh
RUN git clone https://github.com/hlissner/zsh-autopair.git /test/autopair
RUN echo "source /test/autopair/autopair.zsh" > /root/.zshrc
RUN chsh -s /bin/zsh
ENTRYPOINT ["zsh"]

Put it in some test folder, then run in the same folder:

docker build -t issue13 .
docker run -it issue13

You'll get a zsh (version 5.1.1) prompt with your plugin enabled. Try to launch mc and notice significant delay while mc waits for prompt to be usable, it gives up after timeout (afaik it's 10 seconds) and starts without subshell. With working subshell you should be able to hit CtrlO and get into regular subshell's prompt, without it mc shows just what was in console previously and returns to mc with any key press.

Also you can grep https://github.com/MidnightCommander/mc/blob/4.8.15/src/subshell.c for ZSH to see how mc spawns a subshell.

hlissner commented 6 years ago

That's perfect!

Turns out mc chokes when autopair binds the space key. That said, autopair doesn't do anything for mc's subshell in either case, so may as well disable autopair altogether in mc sessions. Try adding this to your .zshrc:

AUTOPAIR_INHIBIT_INIT=$MC_SID

If that works out, I'll be happy to integrate it into zsh-autopair.

In the meantime, I'll look into getting around binding the space key, if at all.

z0rc commented 6 years ago

Errm, this just disables autopair under mc subshell. In my case it effectively means I won't be using autopair, because most of my shells tend to be launched from mc.

As I pointed before, there was commit, that worked fine by me. Also everything was working fine before changes related to this issue landed into master.

z0rc commented 6 years ago

Sorry, but this won't work either, different behavior depending whether it's mc subshell or not is a terrible UX. If you don't want to dig into mc case, please provide option to disable space expansion everywhere.

hlissner commented 6 years ago

Ah, you're right. I'm overengineering!

The canonical way to disable space expansion is unset "AUTOPAIR_PAIRS[ ]". I think that (and a mention in the README) should be enough.

I'll close this thread since the OP's issue is resolved.

z0rc commented 6 years ago

Thanks!

kutsan commented 6 years ago

Now, I realized that my backspace widget also isn't working. I bet if I bind something to quote character, those won't work either. Can you do this for other all other keys? (Should I open a new issue for this, tell me if necessary.)

hlissner commented 6 years ago

@kutsan Please open another issue for that. That said, I just added a fallback for backspace. I'm going to need a more general solution for the other keys. I'll look into this over the weekend.

kutsan commented 6 years ago

Just for the sake of availability, new issue located at https://github.com/hlissner/zsh-autopair/issues/14.