olets / zsh-abbr

The zsh manager for auto-expanding abbreviations, inspired by fish. ~13,000 unique cloners as of May '24, 580+ Homebrew installs 6/23-6/24
https://zsh-abbr.olets.dev
Other
517 stars 18 forks source link

fix(import aliases): strip quotes on multi-word expansions #15

Closed allisio closed 4 years ago

allisio commented 4 years ago

A "picture" is worth a thousand words:

asciicast

In essence, abbr --import-aliases currently interprets the single quotes surrounding multiple-word expansions a little too literally.

The proposed patch simply feeds each imported alias through a sed command that removes the quotes before passing the alias to :add(). The alias shell builtin always supplies such expansions in single quotes and it's exceedingly unlikely that an alias would contain ', so the command should be "as simple as possible, but no simpler".

olets commented 4 years ago

Thanks for catching that and for the contribution!

Mind turning on "allow maintainer edits" for this PR (checkbox in the right sidebar)? I found one additional buggy case, and am switching from sed to zsh param expansion

allisio commented 4 years ago

I have Allow edits from maintainers ticked, and I'm curious to see how you're able to handle this without shelling out. I looked where I knew to in search of the right incantation before giving in to sed.

olets commented 4 years ago

Ah yep thought GitHub surfaced the "maintainer you can edit" in the UI, but no.

Pushed! I think that will work, give it a try. I admit I "cheated" on the subshell — thought I'd do something with ${(Q)…} but that was losing other quotes and echo was just so readable

The case that wasn't covered was single quotes in a double-quoted alias - alias ab="a 'b'".


Note to self: these tests could be stronger — they could miss quotation errors. I manually verified

alias abbr_1=a
alias abbr_2="a b"
alias abbr_3='a b'
alias abbr_4='a "b"'
alias abbr_5="a 'b'"
alias abbr_6="a \"b\""
abbr --import-aliases
# try the abbreviations
allisio commented 4 years ago

That seems to have done the trick!

I sourced my aliases config file (which has a mix of single- and double-quoted expansions) and all of them appear to have imported correctly. I wouldn't say any of them are "pathological", but those tests look like they pretty much cover every reasonable case. Looks like my usage of zsh-abbr is gonna be smooth sailing from here on, but of course that's a good problem to have.

I'll let you know if and when that stops being the case. Thanks for the great library! :+1: