request for hack: readline fixes

request for hack: readline fixes

[Andy Wingo]

Hello list,

It would be really great if someone could get to the bottom of our
readline support and fix a couple things.

 * Tab-completion: Works great for functions, not so much for arguments?

   If at the repl you type `(string-' and press tab, you get a list of
   symbols bound in the current module that start with string-. Of
   course it would be nice to restrict these to procedures, but hey. But
   if you type `(string-ref fo' and press TAB, you get nothing, not even
   if `foo' is bound. Bummer, no? It's been this way for at least 10
   years but I know that readline can do the right thing. Please fix?

 * History: Expression-oriented, please!

   It would be great if going back in the history cycled through entire
   expressions, not line-by-line. Again I know that readline can do
   this. Please fix? :)

Thanks! It's an isolated hack, but many of us will thank you for fixing
it!

Andy
-- 
http://wingolog.org/

Re: request for hack: readline fixes

[Jose A. Ortega Ruiz]

[-- Attachment #1: Type: text/plain, Size: 1388 bytes --]

On Tue, Jun 01 2010, Andy Wingo wrote:

> Hello list,
>
> It would be really great if someone could get to the bottom of our
> readline support and fix a couple things.
>
>  * Tab-completion: Works great for functions, not so much for arguments?
>
>    If at the repl you type `(string-' and press tab, you get a list of
>    symbols bound in the current module that start with string-. Of
>    course it would be nice to restrict these to procedures, but hey. But
>    if you type `(string-ref fo' and press TAB, you get nothing, not even
>    if `foo' is bound. Bummer, no? It's been this way for at least 10
>    years but I know that readline can do the right thing. Please fix?

I think the attached 1-character patch fixes argument completion.
Offering only callable symbols when completing on the first position is
something i hadn't thought of for geiser: i'm probably implementing it
there first and will try to backport the implementation to guile.

>
>  * History: Expression-oriented, please!
>
>    It would be great if going back in the history cycled through entire
>    expressions, not line-by-line. Again I know that readline can do
>    this. Please fix? :)

I'll try to find the time for that too. In the meantime, just use
geiser's REPL :)

Cheers,
jao
-- 
A lot of people have my books on their bookshelves.
That's the problem, they need to read them. -- Don Knuth


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: readline.patch --]
[-- Type: text/x-diff, Size: 468 bytes --]

diff --git a/guile-readline/readline.c b/guile-readline/readline.c
index 2d3617d..ed524da 100644
--- a/guile-readline/readline.c
+++ b/guile-readline/readline.c
@@ -554,7 +554,7 @@ scm_init_readline ()
 #else  
   rl_completion_entry_function = (Function*) completion_function;
 #endif
-  rl_basic_word_break_characters = "\t\n\"'`;()";
+  rl_basic_word_break_characters = " \t\n\"'`;()";
   rl_readline_name = "Guile";
 
   reentry_barrier_mutex = scm_make_mutex ();

Re: request for hack: readline fixes

[Neil Jerram]

[-- Attachment #1: Type: text/plain, Size: 331 bytes --]

Andy Wingo <wingo@pobox.com> writes:

>  * History: Expression-oriented, please!
>
>    It would be great if going back in the history cycled through entire
>    expressions, not line-by-line. Again I know that readline can do
>    this. Please fix? :)

The attached seems to work for me.  Does it look OK?

Regards,
        Neil


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Expression-oriented-readline-history.patch --]
[-- Type: text/x-diff, Size: 3873 bytes --]

From 77c260435092f9bbfba17b6b2b4e4c8b1c5623d9 Mon Sep 17 00:00:00 2001
From: Neil Jerram <neil@ossau.uklinux.net>
Date: Sat, 30 Oct 2010 16:28:54 +0100
Subject: [PATCH] Expression-oriented readline history

* guile-readline/ice-9/readline.scm (make-readline-port): Instead of
  calling add-history after every %readline call, do it only when
  starting a new read.  Other times, append the line just read to an
  internal buffer.
---
 guile-readline/ice-9/readline.scm |   43 +++++++++++++++++++++++++------------
 1 files changed, 29 insertions(+), 14 deletions(-)

diff --git a/guile-readline/ice-9/readline.scm b/guile-readline/ice-9/readline.scm
index 4879bab..36f805f 100644
--- a/guile-readline/ice-9/readline.scm
+++ b/guile-readline/ice-9/readline.scm
@@ -80,20 +80,35 @@
 (define read-hook #f)
 
 (define (make-readline-port)
-  (make-line-buffered-input-port (lambda (continuation?)
-                                   (let* ((prompt (if continuation?
-                                                      continuation-prompt
-                                                      new-input-prompt))
-                                          (str (%readline (if (string? prompt)
-                                                              prompt
-                                                              (prompt))
-                                                          input-port
-                                                          output-port
-                                                          read-hook)))
-                                     (or (eof-object? str)
-                                         (string=? str "")
-                                         (add-history str))
-                                     str))))
+  (let ((history-buffer #f))
+    (make-line-buffered-input-port (lambda (continuation?)
+                                     ;; When starting a new read, add
+                                     ;; the previously read expression
+                                     ;; to the history.
+                                     (if (and (not continuation?)
+                                              history-buffer)
+                                         (begin
+                                           (add-history history-buffer)
+                                           (set! history-buffer #f)))
+                                     ;; Set up prompts and read a line.
+                                     (let* ((prompt (if continuation?
+                                                        continuation-prompt
+                                                        new-input-prompt))
+                                            (str (%readline (if (string? prompt)
+                                                                prompt
+                                                                (prompt))
+                                                            input-port
+                                                            output-port
+                                                            read-hook)))
+                                       (or (eof-object? str)
+                                           (string=? str "")
+                                           (set! history-buffer
+                                                 (if history-buffer
+                                                     (string-append history-buffer
+                                                                    " "
+                                                                    str)
+                                                     str)))
+                                       str)))))
 
 ;;; We only create one readline port.  There's no point in having
 ;;; more, since they would all share the tty and history ---
-- 
1.7.1

Re: request for hack: readline fixes

[Neil Jerram]

Neil Jerram <neil@ossau.uklinux.net> writes:

> Subject: [PATCH] Expression-oriented readline history

A couple more thoughts occurred to me.

1. I should add a `sans-surrounding-whitespace' call to that code (when
appending to the history buffer).

2. An interesting different approach would be not to do the add-history
here (i.e. in make-readline-port) at all, but instead in
readline-repl-reader, by writing the expression that was read into a
string, and adding that string with add-history.  That would give a less
verbatim kind of history, e.g. (I guess) "'a" would appear in the
history as "(quote a)".  But it would cope better with multiple
expressions being entered at the same readline prompt.

(With the posted patch, if the lines of input were

(1
2) (3
4)

I think there would be one line added to the history:

(1 2) (3 4)

)

Perhaps we could provide both approaches, with a switch to choose
between them?  (Or even all three, if we wanted to keep the current
behaviour too.)

         Neil

Re: request for hack: readline fixes

[Andy Wingo]

Hi Neil,

On Sun 31 Oct 2010 00:11, Neil Jerram <neil@ossau.uklinux.net> writes:

> Neil Jerram <neil@ossau.uklinux.net> writes:
>
>> Subject: [PATCH] Expression-oriented readline history

Awesome! Applied, with gusto :)

> 2. An interesting different approach would be not to do the add-history
> here (i.e. in make-readline-port) at all, but instead in
> readline-repl-reader, by writing the expression that was read into a
> string, and adding that string with add-history.  That would give a less
> verbatim kind of history, e.g. (I guess) "'a" would appear in the
> history as "(quote a)".  But it would cope better with multiple
> expressions being entered at the same readline prompt.

Hummmm. I would like this, but we would have to use the pretty-printer
for the current language, so that it would work for ecmascript too.

Andy
-- 
http://wingolog.org/