From 115892ccd270cce4f6b1a90810d7bf2b5bb5b3a2 Mon Sep 17 00:00:00 2001 From: Fabian Homborg Date: Fri, 29 Nov 2019 20:09:02 +0100 Subject: [PATCH] alias: Use `read --tokenize` This did some weird unescaping to try to extract the first word. So we're now more likely to be *correct*, and the alias benchmark is about 20% *faster*. Call it a win-win. --- benchmarks/benchmarks/aliases.fish | 16 +++------------- share/functions/alias.fish | 16 +++------------- tests/checks/alias.fish | 2 ++ 3 files changed, 8 insertions(+), 26 deletions(-) diff --git a/benchmarks/benchmarks/aliases.fish b/benchmarks/benchmarks/aliases.fish index 0eb865842..e6a366685 100644 --- a/benchmarks/benchmarks/aliases.fish +++ b/benchmarks/benchmarks/aliases.fish @@ -44,22 +44,12 @@ function alias --description 'Creates a function wrapping a command' return 1 end - # Extract the first command from the body. This is supposed to replace all non-escaped (i.e. - # preceded by an odd number of `\`) spaces with a newline so it splits on them. See issue #2220 - # for why the following borderline incomprehensible code exists. - set -l tmp (string replace -ra -- "([^\\\ ])((\\\\\\\)*) " '$1\n' $body) - set first_word (string trim -- $tmp[1]) - # If the user does something like `alias x 'foo; bar'` we need to strip the semicolon. - set base_command (string trim -c ';' -- $first_word) - if set -q tmp[2] - set body $tmp[2..-1] - else - set body - end + # Extract the first command from the body. + printf '%s\n' $body | read -lt first_word body # Prevent the alias from immediately running into an infinite recursion if # $body starts with the same command as $name. - if test $base_command = $name + if test $first_word = $name if contains $name (builtin --names) set prefix builtin else diff --git a/share/functions/alias.fish b/share/functions/alias.fish index 0651cc6ac..ea3c15863 100644 --- a/share/functions/alias.fish +++ b/share/functions/alias.fish @@ -44,22 +44,12 @@ function alias --description 'Creates a function wrapping a command' return 1 end - # Extract the first command from the body. This is supposed to replace all non-escaped (i.e. - # preceded by an odd number of `\`) spaces with a newline so it splits on them. See issue #2220 - # for why the following borderline incomprehensible code exists. - set -l tmp (string replace -ra -- "([^\\\ ])((\\\\\\\)*) " '$1\n' $body) - set first_word (string trim -- $tmp[1]) - # If the user does something like `alias x 'foo; bar'` we need to strip the semicolon. - set base_command (string trim -c ';' -- $first_word) - if set -q tmp[2] - set body $tmp[2..-1] - else - set body - end + # Extract the first command from the body. + printf '%s\n' $body | read -lt first_word body # Prevent the alias from immediately running into an infinite recursion if # $body starts with the same command as $name. - if test $base_command = $name + if test $first_word = $name if contains $name (builtin --names) set prefix builtin else diff --git a/tests/checks/alias.fish b/tests/checks/alias.fish index fb3bb3f7c..569ee125e 100644 --- a/tests/checks/alias.fish +++ b/tests/checks/alias.fish @@ -10,9 +10,11 @@ my_alias alias a-2='echo "hello there"' +alias foo '"a b" c d e' # Bare `alias` should list the aliases we have created and nothing else # We have to exclude two aliases because they're an artifact of the unit test # framework and we can't predict the definition. alias | grep -Ev '^alias (fish_indent|fish_key_reader) ' # CHECK: alias a-2 'echo "hello there"' +# CHECK: alias foo '"a b" c d e' # CHECK: alias my_alias 'foo; and echo foo ran'