Items are added to history too early

[lilyball]

New commands are added to the history before the command itself is evaluated. This means that $history[1] expands to the current command, as does history | head -n1. I've been informed that Fish v2.1.2 does not do this (they expand to the previously-executed line instead), and it also seems rather surprising for a command to be considered "history" when it hasn't executed yet.

I believe this was caused by aa1b065.

/cc @ridiculousfish

[ridiculousfish]

> echo $history[1]
echo $history[1]

Whoa, meta.

[ridiculousfish]

Nice find by the way.

[ridiculousfish]

In order to make commands like reboot appear in the history, we need to add them before the command is executed. But of course the command itself should not be able to see them. So I guess we need to temporarily hide the most recent item while he command executes.

[snnw]

+1 I've seen this as well, but didn't know it was a bug. I'll whip up a
patch of no one beats me to it.

On Sat, 18 Apr 2015 05:54 ridiculousfish notifications@github.com wrote:

In order to make commands like reboot appear in the history, we need to
add them before the command is executed. But of course the command itself
should not be able to see them. So I guess we need to temporarily hide the
most recent item while he command executes.

—
Reply to this email directly or view it on GitHub
#2028 (comment)
.

[ridiculousfish]

These are some fixes I've considered:

1. Add a is_pending bool to history_item_t. I dislike this because, after we execute the command, we need to clear the pending flag, and there's no easy way to do that except by iterating through all of the new items.
2. Add a first_pending_new_item_index offset in history_t, similar to first_unwritten_new_item_index. Then history_t::item_at_index returns values counting from this index, instead of from the end of the item array. This is not so bad, except a little more complex than I'd like.
3. Add a pending_items array, similar to new_items. This avoids having to change item_at_index. This raises the question of how the pending item is handled during save. Probably the simplest thing is to just not save pending items, until we mark them as not pending. We already have some hacks for commands reboot and exit to trigger an immediate save; we can extend that to not mark the item as pending. This seems like the cleanest approach.

[ridiculousfish]

I noticed that a simple flag was sufficient. Tests added too. Thanks for reporting this!

[snnw]

Nice fix! :D

On Mon, 20 Apr 2015 11:04 ridiculousfish notifications@github.com wrote:

I noticed that a simple flag was sufficient. Thanks for reporting this.

—
Reply to this email directly or view it on GitHub
#2028 (comment)
.