From: Takao Kouji <kouji@...7.net>
Date: 2008-07-19T23:05:16+09:00
Subject: [ruby-dev:35592] Re: [Ruby 1.8 - Bug #212] Issues with Readline in Mac OS X

高尾宏治です。

On 2008/07/19, at 9:24, Takao Kouji wrote:
> ということで、Readline::HISTORYに対する単体テストを作成しました。
> Readline::HISTORYの全てのメソッドのテストを記述しています。

今回の問題に対応するためのパッチを作成しました。
# テストがあるっていいですね。

パッチの概要を次に述べます。
* GNU Readline を使用した場合の挙動は一切変えません。(変えないように実装したつもりです。)
* パッチの適用前のReadlineモジュールには、
  EditLine(libedit) を使用する場合のみ次の問題があります。
  * libedit のバージョン (2.9より上?) によっては、
    history_get 関数で指定するインデックスは 0 から始まる。
    history_base からではない。
    これが原因で、 Readline::HISTORY ではユーザからの最初の入力を取得できない。
  * history_get 関数で -1 を指定すると 0 番目の history を取得する。
    GNU Readline では NULL である。
    これが原因で、 Readline::HISTORY[-100] では IndexError 例外が発生しない。
  * replace_history_entry 関数で -1 を指定すると 0 番目の history を操作する。
    GNU Readline では操作できない。
    これが原因で、 Readline::HISTORY[-100] = "abc" などにより
    0 番目の history を操作してしまう。
* 上記を回避するため、次のことを行う。
  * Init_readline 関数で libedit かどうかを判定する。
  * libedit であれば、 history の操作時の最初のインデックスの番号が
    history_base か 0 かを調査する。
  * 上記の情報を hist_get (Readline::HISTORY[] -> string | nil) で使用する。
  * また、 hist_get や hist_set (Readline::HISTORY[index] = string) で
    Ruby で index が負の値かどうかチェックする。

なお、パッチの概要は前田さんとも情報共有できていると思うのですが、
実際のパッチの内容まではまだ見ていただいてない状態です。
お手数ですが、確認をお願いします。 > 前田さん

私の手元の環境である Mac OSXのlibedit、
Mac OSX で MacPorts で入れた GNU Readline 5.2 でしか
動作を確認できていません。
そこで、 [ruby-dev:35553] で報告があった Ubuntu 8.04 の libedit や
Mac OSX 10.4 の libedit で試していただける方を募集します。

試す方法を以下に書きます。
1. ruby の trunk を用意する。
2. このメールのパッチと [ruby-dev:35588] のパッチを当てる。
   $ cd /path/to/ruby/trunk
   $ patch -p0 < /path/to/patch
3. configure に --with-libedit を付けて実行する。
   $ ./configure --with-libedit
4. make
5. 単体テストの実行。
   $ ./ruby -I.ext/<arch dir> -rreadline \
   test/readline/test_readline_history.rb

上記のテストがパスできれば OK です。
パスできなかった場合は教えてください。がんばります。

以下、パッチです。
----- ここから -----
Index: readline.c
===================================================================
--- readline.c	(revision 18084)
+++ readline.c	(working copy)
@@ -29,6 +29,8 @@
 
 static VALUE mReadline;
 
+#define EDIT_LINE_LIBRARY_VERSION "EditLine wrapper"
+
 #define COMPLETION_PROC "completion_proc"
 #define COMPLETION_CASE_FOLD "completion_case_fold"
 static ID completion_proc, completion_case_fold;
@@ -43,6 +45,8 @@
 # define rl_completion_matches completion_matches
 #endif
 
+static int (*history_get_offset_func)(int);
+
 static char **readline_attempted_completion_function(const char *text,
                                                      int start, int end);
 
@@ -505,10 +509,22 @@
     return rb_str_new2("HISTORY");
 }
 
+static int
+history_get_offset_history_base(int offset)
+{
+    return history_base + offset;
+}
+
+static int
+history_get_offset_0(int offset)
+{
+    return offset;
+}
+
 static VALUE
 hist_get(VALUE self, VALUE index)
 {
-    HIST_ENTRY *entry;
+    HIST_ENTRY *entry = NULL;
     int i;
 
     rb_secure(4);
@@ -516,7 +532,9 @@
     if (i < 0) {
         i += history_length;
     }
-    entry = history_get(history_base + i);
+    if (i >= 0) {
+	entry = history_get(history_get_offset_func(i));
+    }
     if (entry == NULL) {
 	rb_raise(rb_eIndexError, "invalid index");
     }
@@ -527,7 +545,7 @@
 hist_set(VALUE self, VALUE index, VALUE str)
 {
 #ifdef HAVE_REPLACE_HISTORY_ENTRY
-    HIST_ENTRY *entry;
+    HIST_ENTRY *entry = NULL;
     int i;
 
     rb_secure(4);
@@ -536,7 +554,9 @@
     if (i < 0) {
         i += history_length;
     }
-    entry = replace_history_entry(i, RSTRING_PTR(str), NULL);
+    if (i >= 0) {
+	entry = replace_history_entry(i, RSTRING_PTR(str), NULL);
+    }
     if (entry == NULL) {
 	rb_raise(rb_eIndexError, "invalid index");
     }
@@ -581,7 +601,7 @@
     entry = remove_history(index);
     if (entry) {
         val = rb_tainted_str_new2(entry->line);
-        free(entry->line);
+        free((void *) entry->line);
         free(entry);
         return val;
     }
@@ -624,7 +644,7 @@
 
     rb_secure(4);
     for (i = 0; i < history_length; i++) {
-        entry = history_get(history_base + i);
+        entry = history_get(history_get_offset_func(i));
         if (entry == NULL)
             break;
 	rb_yield(rb_tainted_str_new2(entry->line));
@@ -793,8 +813,17 @@
     rb_define_singleton_method(ucomp, "call",
 			       username_completion_proc_call, 1);
     rb_define_const(mReadline, "USERNAME_COMPLETION_PROC", ucomp);
+    history_get_offset_func = history_get_offset_history_base;
 #if defined HAVE_RL_LIBRARY_VERSION
     rb_define_const(mReadline, "VERSION", rb_str_new2(rl_library_version));
+    if (strncmp(rl_library_version, EDIT_LINE_LIBRARY_VERSION, 
+		strlen(EDIT_LINE_LIBRARY_VERSION)) == 0) {
+	add_history("1");
+	if (history_get(history_get_offset_func(0)) == NULL) {
+	    history_get_offset_func = history_get_offset_0;
+	}
+	clear_history();
+    }
 #else
     rb_define_const(mReadline, "VERSION", rb_str_new2("2.0 or prior version"));
 #endif