bpo-43172: readline now passes its tests when built against libedit (GH-24499)

bpo-43172: readline now passes its tests when built against libedit.

Existing irreconcilable API differences remain in readline.get_begidx
and readline.get_endidx behavior based on libreadline vs libedit use.
A note about that has been documented.

Doc/library/readline.rst

@@ -258,7 +258,9 @@ with a custom completer, a different set of word delimiters should be set.
    Get the beginning or ending index of the completion scope.
    These indexes are the *start* and *end* arguments passed to the
    :c:data:`rl_attempted_completion_function` callback of the
-   underlying library.
+   underlying library.  The values may be different in the same
+   input editing scenario based on the underlying C readline implemtation.
+   Ex: libedit is known to behave differently than libreadline.
 
 
 .. function:: set_completer_delims(string)

Lib/test/test_readline.py

@@ -102,8 +102,15 @@ def test_write_read_append(self):
 
         # test 'no such file' behaviour
         os.unlink(hfilename)
-        with self.assertRaises(FileNotFoundError):
+        try:
             readline.append_history_file(1, hfilename)
+        except FileNotFoundError:
+            pass  # Some implementations return this error (libreadline).
+        else:
+            os.unlink(hfilename)  # Some create it anyways (libedit).
+            # If the file wasn't created, unlink will fail.
+        # We're just testing that one of the two expected behaviors happens
+        # instead of an incorrect error.
 
         # write_history_file can create the target
         readline.write_history_file(hfilename)
@@ -228,7 +235,17 @@ def display(substitution, matches, longest_match_length):
         output = run_pty(script, input)
         self.assertIn(b"text 't\\xeb'\r\n", output)
         self.assertIn(b"line '[\\xefnserted]|t\\xeb[after]'\r\n", output)
-        self.assertIn(b"indexes 11 13\r\n", output)
+        if sys.platform == "darwin" or not is_editline:
+            self.assertIn(b"indexes 11 13\r\n", output)
+            # Non-macOS libedit does not handle non-ASCII bytes
+            # the same way and generates character indices
+            # rather than byte indices via get_begidx() and
+            # get_endidx().  Ex: libedit2 3.1-20191231-2 on Debian
+            # winds up with "indexes 10 12".  Stemming from the
+            # start and end values calls back into readline.c's
+            # rl_attempted_completion_function = flex_complete with:
+            # (11, 13) instead of libreadline's (12, 15).
+
         if not is_editline and hasattr(readline, "set_pre_input_hook"):
             self.assertIn(b"substitution 't\\xeb'\r\n", output)
             self.assertIn(b"matches ['t\\xebnt', 't\\xebxt']\r\n", output)

Misc/NEWS.d/next/Library/2021-02-10-06-00-53.bpo-43172.ZMCJni.rst

@@ -0,0 +1,4 @@
+The readline module now passes its tests when built directly against
+libedit. Existing irreconcilable API differences remain in
+:func:`readline.get_begidx` and :func:`readline.get_endidx` behavior based
+on libreadline vs libedit use.

Modules/clinic/readline.c.h

@@ -384,7 +384,7 @@ PyDoc_STRVAR(readline_remove_history_item__doc__,
 "remove_history_item($module, pos, /)\n"
 "--\n"
 "\n"
-"Remove history item given by its position.");
+"Remove history item given by its zero-based position.");
 
 #define READLINE_REMOVE_HISTORY_ITEM_METHODDEF    \
     {"remove_history_item", (PyCFunction)readline_remove_history_item, METH_O, readline_remove_history_item__doc__},
@@ -412,7 +412,9 @@ PyDoc_STRVAR(readline_replace_history_item__doc__,
 "replace_history_item($module, pos, line, /)\n"
 "--\n"
 "\n"
-"Replaces history item given by its position with contents of line.");
+"Replaces history item given by its position with contents of line.\n"
+"\n"
+"pos is zero-based.");
 
 #define READLINE_REPLACE_HISTORY_ITEM_METHODDEF    \
     {"replace_history_item", (PyCFunction)(void(*)(void))readline_replace_history_item, METH_FASTCALL, readline_replace_history_item__doc__},
@@ -563,7 +565,7 @@ PyDoc_STRVAR(readline_get_history_item__doc__,
 "get_history_item($module, index, /)\n"
 "--\n"
 "\n"
-"Return the current contents of history item at index.");
+"Return the current contents of history item at one-based index.");
 
 #define READLINE_GET_HISTORY_ITEM_METHODDEF    \
     {"get_history_item", (PyCFunction)readline_get_history_item, METH_O, readline_get_history_item__doc__},
@@ -683,4 +685,4 @@ readline_redisplay(PyObject *module, PyObject *Py_UNUSED(ignored))
 #ifndef READLINE_CLEAR_HISTORY_METHODDEF
     #define READLINE_CLEAR_HISTORY_METHODDEF
 #endif /* !defined(READLINE_CLEAR_HISTORY_METHODDEF) */
-/*[clinic end generated code: output=cb44f391ccbfb565 input=a9049054013a1b77]*/
+/*[clinic end generated code: output=f7d390113b27989f input=a9049054013a1b77]*/

Modules/readline.c

@@ -67,7 +67,8 @@ extern char **completion_matches(char *, CPFunction *);
 static int using_libedit_emulation = 0;
 static const char libedit_version_tag[] = "EditLine wrapper";
 
-static int libedit_history_start = 0;
+static int8_t libedit_history_start = 0;
+static int8_t libedit_append_replace_history_offset = 0;
 
 #ifdef HAVE_RL_COMPLETION_DISPLAY_MATCHES_HOOK
 static void
@@ -320,7 +321,8 @@ readline_append_history_file_impl(PyObject *module, int nelements,
         filename_bytes = NULL;
         filename = NULL;
     }
-    errno = err = append_history(nelements, filename);
+    errno = err = append_history(
+        nelements - libedit_append_replace_history_offset, filename);
     if (!err && _history_length >= 0)
         history_truncate_file(filename, _history_length);
     Py_XDECREF(filename_bytes);
@@ -592,12 +594,12 @@ readline.remove_history_item
     pos as entry_number: int
     /
 
-Remove history item given by its position.
+Remove history item given by its zero-based position.
 [clinic start generated code]*/
 
 static PyObject *
 readline_remove_history_item_impl(PyObject *module, int entry_number)
-/*[clinic end generated code: output=ab114f029208c7e8 input=c8520ac3da50224e]*/
+/*[clinic end generated code: output=ab114f029208c7e8 input=f248beb720ff1838]*/
 {
     HIST_ENTRY *entry;
 
@@ -626,12 +628,14 @@ readline.replace_history_item
     /
 
 Replaces history item given by its position with contents of line.
+
+pos is zero-based.
 [clinic start generated code]*/
 
 static PyObject *
 readline_replace_history_item_impl(PyObject *module, int entry_number,
                                    PyObject *line)
-/*[clinic end generated code: output=f8cec2770ca125eb input=b7ccef0780ae041b]*/
+/*[clinic end generated code: output=f8cec2770ca125eb input=368bb66fe5ee5222]*/
 {
     PyObject *encoded;
     HIST_ENTRY *old_entry;
@@ -645,7 +649,9 @@ readline_replace_history_item_impl(PyObject *module, int entry_number,
     if (encoded == NULL) {
         return NULL;
     }
-    old_entry = replace_history_entry(entry_number, PyBytes_AS_STRING(encoded), (void *)NULL);
+    old_entry = replace_history_entry(
+        entry_number + libedit_append_replace_history_offset,
+        PyBytes_AS_STRING(encoded), (void *)NULL);
     Py_DECREF(encoded);
     if (!old_entry) {
         PyErr_Format(PyExc_ValueError,
@@ -786,12 +792,12 @@ readline.get_history_item
     index as idx: int
     /
 
-Return the current contents of history item at index.
+Return the current contents of history item at one-based index.
 [clinic start generated code]*/
 
 static PyObject *
 readline_get_history_item_impl(PyObject *module, int idx)
-/*[clinic end generated code: output=83d3e53ea5f34b3d input=63fff0c3c4323269]*/
+/*[clinic end generated code: output=83d3e53ea5f34b3d input=8adf5c80e6c7ff2b]*/
 {
     HIST_ENTRY *hist_ent;
 
@@ -1191,6 +1197,22 @@ setup_readline(readlinestate *mod_state)
     } else {
         libedit_history_start = 1;
     }
+    /* Some libedit implementations use 1 based indexing on
+     * replace_history_entry where libreadline uses 0 based.
+     * The API our module presents is supposed to be 0 based.
+     * It's a mad mad mad mad world.
+     */
+    {
+        add_history("2");
+        HIST_ENTRY *old_entry = replace_history_entry(1, "X", NULL);
+        _py_free_history_entry(old_entry);
+        HIST_ENTRY *item = history_get(libedit_history_start);
+        if (item && item->line && strcmp(item->line, "X")) {
+            libedit_append_replace_history_offset = 0;
+        } else {
+            libedit_append_replace_history_offset = 1;
+        }
+    }
     clear_history();
 
     using_history();