Install instructions for Fish are wrong

[lilyball]

The install instructions for Fish says to define

function fuck
    eval (thefuck (history | head -n1))
end

This won't work, because history will include the currently-executing command in its output, which means that this will always evaluate to eval (thefuck fuck). It's also rather wasteful, as history dumps a bunch of history. There's also the curious property wherein if thefuck emits multiple lines, they'll end up being joined into a single line, as command substitution splits lines into arguments, and eval joins multiple arguments with spaces instead of newlines.

Both of these issues can be fixed by rewriting it as

function fuck
    thefuck $history[2] | source
end

[nvbn]

Thanks.

[zenoamaro]

@kballard, the bits about waste, quoting and sourcing instead of evaling are spot-on, but are you sure about history also including the currently running command?

$ echo foo
foo
$ echo (history | head -n1)
echo foo

Conversely, I find that history[2] yields the second-to-last command:

$ echo foo
foo
$ echo bar
bar
$ echo $history[2]
echo foo

This is with fish v2.1.1,

[lilyball]

That's odd. My several-months-old dev version of Fish 2.1.1, and my just-installed tip-of-master version of Fish 2.1.2, both behave as I described:

> echo foo
foo
> echo (history | head -n1)
echo (history | head -n1)
> echo foo
foo
> echo bar
bar
> echo $history[2]
echo bar

[zenoamaro]

I can see mentions of $history[1] in various threads:

• Get the last command when when capturing fish_prompt event fish-shell/fish-shell#984
• support history expansion (e.g., bash's !!, bang bang, and !$, bang dollar, tokens) fish-shell/fish-shell#288

Where ridiculousfish themself agree that it should return the last
executed command.

May be a difference in arch/plugins/etc? For comparison, this is Fish
v2.1.1 installed with Homebrew on OSX.

[lilyball]

I'm also running OS X.

I'm wondering now if maybe master has a bug where it's adding the command to history too eagerly. Note that master actually has a lot of changes from 2.1.2 (I believe master has been considered to be equivalent to at least 2.2 for a long time, and 2.1.1 / 2.1.2 were just minor changes from 2.1.0).

Given the state of things at the moment, I guess the install instructions should be changed to use $history[1] instead of $history[2] until this can be tracked down.

[lilyball]

Yep, the commit referenced in that issue fish-shell/fish-shell#984 (aa1b065dd1c03f7c0867d002badcf0dc88feb3c4) is the culprit. Building fish from before that commit expands $history[1] to the previous command instead of the current one. That commit is only on master, it's not in v2.1.2.

[zenoamaro]

Here is a repeatable test on a completely pristine machine from instantcloud.io with even older fish 2.1.0:

[lilyball]

I've filed fish-shell/fish-shell#2028 about this issue.

In the meantime, @nvbn, it seems clear that the instructions need to be changed to say $history[1] instead of $history[2] (but other than that, the new function is better than the old).

[zenoamaro]

I agree, if not only for the quoting and eval issues.

Performance is also (predictably) way better:

$ time -p fish -c 'for a in (seq 1000); history | head -n1 > /dev/null; end'
real         3.25
user         2.06
sys          1.58

$ time -p fish -c 'for a in (seq 1000); echo $history[1] > /dev/null; end'
real         1.32
user         0.65
sys          0.84