Configuration Information [Automatically generated, do not change]:
Machine: x86_64
OS: linux-gnu
Compiler: gcc
Compilation CFLAGS: -g -O2 -fdebug-prefix-map=/build/bash-2bxm7h/bash-5.0=.
-fstack-protector-strong -Wformat -Werror=format-security -Wall
-Wno-parentheses -Wno-format-security
uname output: Linux dark-matter 4.19.0-8-amd64 #1 SMP Debian 4.19.98-1
(2020-01-26) x86_64 GNU/Linux
Machine Type: x86_64-pc-linux-gnu

Bash Version: 5.0
Patch Level: 3
Release Status: release

Description:

  When the function operate-and-get-next (C-o) is used on a multi-line
  command, if the history is full, the next prompt shows not the next
  command but the one after that.

Repeat-By:

  0. If your history is full in your normal shell session (i.e., it
  already contains $HISTSIZE entries), just start from step 1.  Or,
  start a fresh shell with "HISTFILE= HISTSIZE=4 bash --norc -i".

  1. Run the following three commands:

  echo 'a
  b'
  echo c
  echo d

  2. Then go up three places in the history to the two-line echo, and
  hit C-o.

  3. Observe that your next prompt is prefilled with "echo d", not
  "echo c".

Fix:

  The bug appears to be caused by the extra logic that was added to
  set_saved_history to fix this previous bug:

    https://lists.gnu.org/archive/html/bug-bash/2011-09/msg00050.html

  Removing that logic fixes this bug, but naturally reintroduces that one.

  I've attached a patch that fixes this bug without reintroducing the
  old one, and as a bonus the code gets a bit simpler.

  The idea of the patch is that for communicating from
  operate_and_get_next to set_saved_history the information of where
  in the history we want to be, instead of storing a physical offset
  like "where_history ()", we store a logical offset like
  "where_history () + history_base". This allows us to not worry about
  deducing whether the history got shuffled or not, because by design
  the logical offset of a given entry isn't affected when that
  happens.

  The patch also contains tests for both the previous bug (which
  occurs when the multi-line command was read from $HISTFILE) and the
  present one (which occurs when it was entered into the current shell
  session directly).

Thanks,
Greg
From 495f46ec7e1f97e35d24515ccad9b2bd1f8fa60c Mon Sep 17 00:00:00 2001
Message-Id: <495f46ec7e1f97e35d24515ccad9b2bd1f8fa60c.1587271445.git.gnpr...@gmail.com>
From: Greg Price <gnpr...@gmail.com>
Date: Sat, 18 Apr 2020 18:08:55 -0700
Subject: [PATCH] Fix operate-and-get-next skipping command after a multiline
 command.

When the function operate-and-get-next (C-o) is used on a multi-line
command, if the history is full, the next prompt would show not the
next command but the one after that.

E.g. after the following commands, provided your history is already
full (i.e. already contains $HISTSIZE entries):

  $ echo 'a
  b'
  $ echo c
  $ echo d

going up three places to the two-line echo and hitting C-o would
cause the next prompt to be prefilled with "echo d", not "echo c".

The bug appears to be caused by the extra logic that was added to
set_saved_history to fix this previous bug:
  https://lists.gnu.org/archive/html/bug-bash/2011-09/msg00050.html
Just removing that logic would fix this bug, but naturally reintroduce
that one.

Instead, we can fix both while as a bonus making the code a bit
simpler.  For communicating from operate_and_get_next to
set_saved_history the information of where in the history we want to
be, instead of storing a physical offset like "where_history ()", we
store a logical offset like "where_history () + history_base".  This
allows us to not worry about deducing whether the history got shuffled
or not, because by design the logical offset of a given entry isn't
affected when that happens.

Also add tests for both the previous bug (which occurs when the
multi-line command was read from $HISTFILE) and the present one (which
occurs when it was entered into the current shell session directly).
---
 bashline.c          | 36 +++++++++++----------------------
 tests/history.right | 49 +++++++++++++++++++++++++++++++++++++++++++++
 tests/history.tests | 35 ++++++++++++++++++++++++++++++++
 3 files changed, 96 insertions(+), 24 deletions(-)

diff --git bashline.c bashline.c
index cd36b0be5..6202b6941 100644
--- bashline.c
+++ bashline.c
@@ -922,27 +922,20 @@ hostnames_matching (text)
 
 /* The equivalent of the Korn shell C-o operate-and-get-next-history-line
    editing command. */
-static int saved_history_line_to_use = -1;
-static int last_saved_history_line = -1;
-
-#define HISTORY_FULL() (history_is_stifled () && history_length >= history_max_entries)
+static int saved_history_logical_offset = -1;
 
 static int
 set_saved_history ()
 {
-  /* XXX - compensate for assumption that history was `shuffled' if it was
-     actually not. */
-  if (HISTORY_FULL () &&
-      hist_last_line_added == 0 &&
-      saved_history_line_to_use < history_length - 1)
-    saved_history_line_to_use++;
-
-  if (saved_history_line_to_use >= 0)
+  int absolute_offset, count;
+
+  if (saved_history_logical_offset >= 0)
     {
-     rl_get_previous_history (history_length - saved_history_line_to_use, 0);
-     last_saved_history_line = saved_history_line_to_use;
+      absolute_offset = saved_history_logical_offset - history_base;
+      count = where_history () - absolute_offset;
+      rl_get_previous_history (count, 0);
     }
-  saved_history_line_to_use = -1;
+  saved_history_logical_offset = -1;
   rl_startup_hook = old_rl_startup_hook;
   return (0);
 }
@@ -951,18 +944,13 @@ static int
 operate_and_get_next (count, c)
      int count, c;
 {
-  int where;
-
   /* Accept the current line. */
   rl_newline (1, c);
 
-  /* Find the current line, and find the next line to use. */
-  where = rl_explicit_arg ? count : where_history ();
-
-  if (HISTORY_FULL () || (where >= history_length - 1) || rl_explicit_arg)
-    saved_history_line_to_use = where;
-  else
-    saved_history_line_to_use = where + 1;
+  saved_history_logical_offset =
+    rl_explicit_arg ? count
+    /* Find the current line, and find the next line to use. */
+    : where_history () + history_base + 1;
 
   old_rl_startup_hook = rl_startup_hook;
   rl_startup_hook = set_saved_history;
diff --git tests/history.right tests/history.right
index bd3be440c..5b83c466e 100644
--- tests/history.right
+++ tests/history.right
@@ -178,3 +178,52 @@ i
     4  echo g
     5  echo h
 
+
+0
+1
+2
+(left
+mid
+right)
+A
+B
+
+(left
+mid
+right)
+A
+B
+
+(left
+mid
+right)
+A
+B
+
+0
+1
+2
+(left
+mid
+right)
+A
+B
+(left
+mid
+right)
+A
+B
+
+0
+1
+2
+(left
+mid
+right)
+A
+B
+(left
+mid
+right)
+A
+B
diff --git tests/history.tests tests/history.tests
index e7451c7b6..6ad8e90d9 100644
--- tests/history.tests
+++ tests/history.tests
@@ -127,3 +127,38 @@ rm -f $TMPDIR/foohist-*
 
 ${THIS_SH} ./history2.sub
 ${THIS_SH} ./history3.sub
+
+
+HISTFILE=$TMPDIR/newhistory
+export HISTFILE
+
+HISTSIZE=32
+HISTFILESIZE=32
+echo
+set -o history
+history -c
+echo 0
+echo 1
+echo 2
+echo "(left
+mid
+right)"
+echo A
+echo B
+history -w
+set +o history
+
+echo
+printf $'HISTFILE=\n\cRleft\cO\cO\cO\cO\n' | HISTSIZE= ${THIS_SH} --norc -i 2>/dev/null
+echo
+printf $'HISTFILE=\n\cRleft\cO\cO\cO\cO\n' | HISTSIZE=8 ${THIS_SH} --norc -i 2>/dev/null
+
+
+input="$(cat $HISTFILE)
+"$'\cP\cP\cP\cO\cO
+'
+
+echo
+printf "$input" | HISTSIZE= HISTFILE= ${THIS_SH} --norc -i 2>/dev/null
+echo
+printf "$input" | HISTSIZE=6 HISTFILE= ${THIS_SH} --norc -i 2>/dev/null
-- 
2.20.1

Reply via email to