larkery / zsh-histdb

A slightly better history for zsh
MIT License
1.29k stars 75 forks source link

Fix sql_escape for multiline inputs #126

Closed c0deaddict closed 2 years ago

c0deaddict commented 2 years ago

In commit https://github.com/larkery/zsh-histdb/commit/0f6075cee6a11bf38f61eaf842f87e2ce2bbc48f the performance of sql_escape was improved. Unfortunately this new sql_escape breaks on some inputs. If i try to enter the following command, i get the error: sql_escape:print:1: bad option: -t after typing the first t in context:

$ kubectl get pods \
--context

My knowledge of variable substitution magic in zsh/bash is limited, but I found that reverting to the old sql_escape definition with sed does work.

comex commented 2 years ago

The issue doesn't have to do with multiline inputs but with inputs starting with a hyphen. If you search for --context, then after expansion, the new implementation of sql_escape ends up running a command like:

print -r --context

The intent is to print the text --context to stdout (where it gets substituted into another command), but instead --context is interpreted as a set of flags for print itself.

The proper fix is to use --:

 sql_escape () {
-    print -r ${${@//\'/\'\'}//$'\x00'}
+    print -r -- ${${@//\'/\'\'}//$'\x00'}
 }

Alternately, it could be made even faster by avoiding command substitution entirely, as command substitution requires forking.

c0deaddict commented 2 years ago

Thanks for the explanation, that sounds logical! The fix you propose works, i've adjusted the PR to it.

larkery commented 2 years ago

Ah this is very helpful - I have been being annoyed by this for a while but not bothered to fix it. Thank you.