From: Takao Kouji <kouji@...7.net>
Date: 2008-07-17T01:48:38+09:00
Subject: [ruby-dev:35551] [Ruby 1.8 - Bug #212] Issues with Readline in Mac OS X

高尾宏治です。

# Subjectを変えます。

On 2008/07/16, at 15:33, Nobuyoshi Nakada wrote:
> At Wed, 16 Jul 2008 11:45:06 +0900,
> Shugo Maeda wrote in [ruby-dev:35532]:
>> Ruby側でとりあえず対応するという点については異論はないのですが、
>> バグだとしたらバージョンによって挙動が変わったら嫌だなと思いました。
>
> 初期化のときにチェックするなら、バージョンが変わっても気にしなく
> てすむと思います。

初期化の時にチェックするのも良さそうですね。
ただし、Readlineの拡張ライブラリの初期化時に、
毎回 libedit とリンクしているかどうかを確認するのは、
どうかということがあると思います。
これについては、Init_readline()で以下のような処理をすると思うのですが、
対したコストではないため、気にしなくても良いのかもしれません。

  if (strncmp(rl_library_version, "EditLine wrapper", 
              strlen("EditLine wrapper")) == 0) {
      /* libedit の場合の処理 */
  }
  else {
      /* libedit の場合の処理 */
  }

>>>> 高尾君のパッチと大分趣が異なるような気がするのですが、どういう意図でしょうか。
>>>
>>> history_baseの初期値を保存しておいて、そこからの差分で見るという
>>> ことです。
>>
>> 自分がlibeditの挙動を理解していないだけかもしれないのですが、それで上手く
>> 動くのでしょうか。
>> 高尾君のパッチではたんにhistory_baseをhistory_base - 1にしているだけだった
>> ので、libeditではhistory_baseの値が1ずれているとかそういう話なのかなと思った
>> のですが。
>
> そのズレの値を初期状態でとれないかと思ったのですが、初期値はずれ
> ているわけではないとしたら的外れかもしれません。後は、
> remove_history()があるならですが、[ruby-dev:35519]のテストコード
> を実行してみるとかしかないですね。
>
> ついでにいえば、remove_history()やreplace_history_entry()も同じ
> くindexをとるわけですが、こちらは問題ないんでしょうか。

問題がありました。
どうも、ヒストリのインデックスの開始が、
* GNU Readline は 1。(history_base と同じ)
* libedit は 0。
という違いがあるようです。
このため、remove_historyなども同様に修正する必要がありました。
これは以下のコードで確認しました。
----- ここから -----
#include <stdio.h>

#ifdef HAVE_EDITLINE_READLINE_H
#include <editline/readline.h>
#else
#include <readline/readline.h>
#include <readline/history.h>
#endif /* HAVE_EDITLINE_READLINE_H */

int main(int argc, char **argv)
{
  HIST_ENTRY *entry;
  int i;

  using_history();

  printf("history_base: %d\n", history_base);

  add_history("1");
  add_history("2");
  add_history("3");

  printf("added 1, 2, 3 to history\n");
  printf("history_base: %d\n", history_base);
  printf("history_length: %d\n", history_length);

  entry = history_get(history_base);
  printf("history_get(history_base): %s\n", entry->line);
  
  for (i = 0; i <= history_length; i++) {
    entry = history_get(i);
    printf("history_get(%d): %s\n", i, entry ? entry->line : "NULL");
  }
  
  return 0;
}
----- ここまで -----

以下、GNU Readline、Mac OSX の libedit、
libedit の 20080712-2.11 で実行した結果です。
----- ここから -----
---- no212_on-macosx.gnu_readline ----
history_base: 1
added 1, 2, 3 to history
history_base: 1
history_length: 3
history_get(history_base): 1
history_get(0): NULL
history_get(1): 1
history_get(2): 2
history_get(3): 3
---- no212_on-macosx.editline ----
history_base: 1
added 1, 2, 3 to history
history_base: 1
history_length: 3
history_get(history_base): 2
history_get(0): 1
history_get(1): 2
history_get(2): 3
history_get(3): NULL
---- no212_on-macosx.editline_20080712-2.11 ----
history_base: 1
added 1, 2, 3 to history
history_base: 1
history_length: 3
history_get(history_base): 2
history_get(0): 1
history_get(1): 2
history_get(2): 3
history_get(3): NULL
----- ここまで -----

最後にこれまでの話などを考慮したパッチを作成しました。
実行時に libedit であれば index のオフセットを 0 に、
そうでない(GNU Readline であれば) オフセットを history_base に
設定するようにしています。

Mac OSX 10.5.4(昨日10.5.3からアップデートしました)の環境で、
動作を確認しています。

$ ~/local/bin/ruby19trunk -rreadline -ve 'puts Readline::VERSION; Readline::HISTORY.push("1", "2", "3"); Readline::HISTORY.each { |s| puts s }; p Readline::HISTORY[1]; Readline::HISTORY[1] = "A" ; p Readline::HISTORY[1]; Readline::HISTORY.delete_at(0); p Readline::HISTORY[0]; p Readline::HISTORY.length; p Readline::HISTORY.pop; p Readline::HISTORY.pop; p Readline::HISTORY.pop; '
ruby 1.9.0 (2008-07-16 revision 18071) [i686-darwin9.4.0]
EditLine wrapper
1
2
3
"2"
"A"
"A"
2
"3"
"A"
nil

----- ここから -----
Index: ext/readline/readline.c
===================================================================
--- ext/readline/readline.c	(revision 18084)
+++ ext/readline/readline.c	(working copy)
@@ -43,6 +43,8 @@
 # define rl_completion_matches completion_matches
 #endif
 
+static int history_offset = 0;
+
 static char **readline_attempted_completion_function(const char *text,
                                                      int start, int end);
 
@@ -516,7 +518,7 @@
     if (i < 0) {
         i += history_length;
     }
-    entry = history_get(history_base + i);
+    entry = history_get(i + history_offset);
     if (entry == NULL) {
 	rb_raise(rb_eIndexError, "invalid index");
     }
@@ -536,7 +538,7 @@
     if (i < 0) {
         i += history_length;
     }
-    entry = replace_history_entry(i, RSTRING_PTR(str), NULL);
+    entry = replace_history_entry(i + history_offset, RSTRING_PTR(str), NULL);
     if (entry == NULL) {
 	rb_raise(rb_eIndexError, "invalid index");
     }
@@ -578,7 +580,7 @@
     VALUE val;
 
     rb_secure(4);
-    entry = remove_history(index);
+    entry = remove_history(index + history_offset);
     if (entry) {
         val = rb_tainted_str_new2(entry->line);
         free(entry->line);
@@ -624,7 +626,7 @@
 
     rb_secure(4);
     for (i = 0; i < history_length; i++) {
-        entry = history_get(history_base + i);
+        entry = history_get(i + history_offset);
         if (entry == NULL)
             break;
 	rb_yield(rb_tainted_str_new2(entry->line));
@@ -795,6 +797,10 @@
     rb_define_const(mReadline, "USERNAME_COMPLETION_PROC", ucomp);
 #if defined HAVE_RL_LIBRARY_VERSION
     rb_define_const(mReadline, "VERSION", rb_str_new2(rl_library_version));
+    if (strncmp(rl_library_version, "EditLine wrapper", 
+		strlen("EditLine wrapper")) != 0) {
+	history_offset = history_base;
+    }
 #else
     rb_define_const(mReadline, "VERSION", rb_str_new2("2.0 or prior version"));
 #endif