Skip to content

Commit

Permalink
Add full fix for #215 along with a test to reproduce the issue (#218)
Browse files Browse the repository at this point in the history
  • Loading branch information
ddworken authored Jun 9, 2024
1 parent 16a4873 commit 095f4a4
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 14 deletions.
14 changes: 13 additions & 1 deletion client/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3127,8 +3127,20 @@ func TestForceInit(t *testing.T) {
require.NotContains(t, tester.RunInteractiveShell(t, `hishtory export`), "echo foobar")
}

func TestBashOrderingBug(t *testing.T) {
markTestForSharding(t, 15)
defer testutils.BackupAndRestore(t)()
tester := bashTester{}
installHishtory(t, tester, "")

// Trigger a set of steps that cause a weird bug with bash as reported in github.com/ddworken/hishtory/issues/215
captureTerminalOutput(t, tester, []string{"command1 Enter", "command2 Enter", "C-R", "C-C", "command3 Enter", "command4 Enter"})
out := tester.RunInteractiveShell(t, `hishtory export | grep -v pipefail | grep -v '/tmp/client install'`)
testutils.CompareGoldens(t, out, "TestBashOrderingBug-Export")
}

func TestChangeSyncingStatus(t *testing.T) {
markTestForSharding(t, 14)
markTestForSharding(t, 15)
defer testutils.BackupAndRestore(t)()
tester := zshTester{}

Expand Down
29 changes: 16 additions & 13 deletions client/lib/config.sh
Original file line number Diff line number Diff line change
Expand Up @@ -20,21 +20,21 @@ function __hishtory_precommand() {
HISHTORY_START_TIME=`hishtory getTimestamp`
CMD=`history 1`
if ! [ -z "CMD " ] ; then
if [[ "$CMD" != "$LAST_PRESAVED_COMMAND" ]] ; then
# This code with $LAST_PRESAVED_COMMAND and $LAST_SAVED_COMMAND is necessary to work around a quirk of
# bash. With bash, if you run the command `foo` and then press `Control+R`, it will trigger the DEBUG
# signal. And `history 1` will still return `foo` so this will lead to duplicate pre-saves causing entries
# to show up twice. This works around this issue by skipping presaving if the last commadn we presaved
# was identical.
#
# This does lead to a potential correctness bug since it means if someone runs the same non-terminating
# command twice in a row, we won't pre-save the second entry. But this seems reasonably unlikely
# such that it is worth accepting this issue to mitigate the duplicate entries observed in
# https://github.com/ddworken/hishtory/issues/215.
if [[ "$CMD" != "$LAST_PRESAVED_COMMAND" ]] && [[ "$CMD" != "$LAST_SAVED_COMMAND" ]]; then
(hishtory presaveHistoryEntry bash "$CMD" $HISHTORY_START_TIME &) # Background Run
# hishtory presaveHistoryEntry bash "$CMD" $HISHTORY_START_TIME # Foreground Run
fi
fi
# This code with $LAST_PRESAVED_COMMAND is necessary to work around a quirk of bash. With bash,
# if you run the command `foo` and then press `Control+R`, it will trigger the DEBUG signal. And
# `history 1` will still return `foo` so this will lead to duplicate pre-saves causing entries to
# show up twice. This works around this issue by skipping presaving if the last commadn we presaved
# was identical.
#
# This does lead to a potential correctness bug since it means if someone runs the same non-terminating
# command twice in a row, we won't pre-save the second entry. But this seems reasonably unlikely
# such that it is worth accepting this issue to mitigate the duplicate entries observed in
# https://github.com/ddworken/hishtory/issues/215.
LAST_PRESAVED_COMMAND=$CMD
}
trap "__hishtory_precommand" DEBUG
Expand All @@ -50,8 +50,11 @@ function __hishtory_postcommand() {
fi

# Run after every prompt
(hishtory saveHistoryEntry bash $EXIT_CODE "`history 1`" $HISHTORY_START_TIME &) # Background Run
# hishtory saveHistoryEntry bash $EXIT_CODE "`history 1`" $HISHTORY_START_TIME # Foreground Run
CMD=`history 1`
(hishtory saveHistoryEntry bash $EXIT_CODE "$CMD" $HISHTORY_START_TIME &) # Background Run
# hishtory saveHistoryEntry bash $EXIT_CODE "$CMD" $HISHTORY_START_TIME # Foreground Run

LAST_SAVED_COMMAND=$CMD

(hishtory updateLocalDbFromRemote &)
}
Expand Down
4 changes: 4 additions & 0 deletions client/testdata/TestBashOrderingBug-Export
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
command1
command2
command3
command4

0 comments on commit 095f4a4

Please sign in to comment.