pushed to master: extensibility to (ice-9 session)

pushed to master: extensibility to (ice-9 session)

[Andy Wingo]

Hi,

I pushed the following patch to master. Is it OK to push to 1.8 as well?
That way I could drop some modules from guile-lib, and make guile-lib
depend on guile >= 1.8.x.

(Perhaps we can set up a list for patches that get pushed to Guile ?)

Andy

commit 4f7a0504aac215832e99290e31c9944795c5d206
Author: Andy Wingo <wingo@pobox.com>
Date:   Tue Jan 27 13:43:07 2009 +0100

    merge in from guile-lib: add some extensibility to `help'
    
    * ice-9/session.scm (add-value-help-handler!)
      (remove-value-help-handler!, add-name-help-handler!)
      (remove-name-help-handler!): New public interfaces, to allow some basic
      extensibility of the help interface. Merged in from guile-lib's (scheme
      session).

diff --git a/ice-9/session.scm b/ice-9/session.scm
index 1c9f480..6971a78 100644
--- a/ice-9/session.scm
+++ b/ice-9/session.scm
@@ -20,12 +20,61 @@
   :use-module (ice-9 documentation)
   :use-module (ice-9 regex)
   :use-module (ice-9 rdelim)
-  :export (help apropos apropos-internal apropos-fold
-	   apropos-fold-accessible apropos-fold-exported apropos-fold-all
-	   source arity system-module))
+  :export (help
+           add-value-help-handler! remove-value-help-handler!
+           add-name-help-handler! remove-name-help-handler!
+           apropos apropos-internal apropos-fold apropos-fold-accessible
+           apropos-fold-exported apropos-fold-all source arity
+           system-module module-commentary))
 
 \f
 
+(define *value-help-handlers* '())
+
+(define (add-value-help-handler! proc)
+  "Adds a handler for performing `help' on a value.
+
+`proc' will be called as (PROC NAME VALUE). `proc' should return #t to
+indicate that it has performed help, a string to override the default
+object documentation, or #f to try the other handlers, potentially
+falling back on the normal behavior for `help'."
+  (set! *value-help-handlers* (cons proc *value-help-handlers*)))
+
+(define (remove-value-help-handler! proc)
+  "Removes a handler for performing `help' on a value.
+
+See the documentation for `add-value-help-handler' for more
+information."
+  (set! *value-help-handlers* (delete! proc *value-help-handlers*)))
+
+(define (try-value-help name value)
+  (or-map (lambda (proc) (proc name value)) *value-help-handlers*))
+
+
+(define *name-help-handlers* '())
+
+(define (add-name-help-handler! proc)
+  "Adds a handler for performing `help' on a name.
+
+`proc' will be called with the unevaluated name as its argument. That is
+to say, when the user calls `(help FOO)', the name is FOO, exactly as
+the user types it.
+
+The return value of `proc' is as specified in
+`add-value-help-handler!'."
+  (set! *name-help-handlers* (cons proc *name-help-handlers*)))
+
+(define (remove-name-help-handler! proc)
+  "Removes a handler for performing `help' on a name.
+
+See the documentation for `add-name-help-handler' for more
+information."
+  (set! *name-help-handlers* (delete! proc *name-help-handlers*)))
+
+(define (try-name-help name)
+  (or-map (lambda (proc) (proc name)) *name-help-handlers*))
+
+
 ;;; Documentation
 ;;;
 (define help
@@ -45,6 +94,10 @@ You don't seem to have regular expressions installed.\n"))
                                                type x))))
                (cond
 
+                ;; User-specified
+                ((try-name-help name)
+                 => (lambda (x) (if (not (eq? x #t)) (display x))))
+
                 ;; SYMBOL
                 ((symbol? name)
                  (help-doc name
@@ -60,10 +113,12 @@ You don't seem to have regular expressions installed.\n"))
                 ((and (list? name)
                       (= (length name) 2)
                       (eq? (car name) 'unquote))
-                 (cond ((object-documentation
-                         (local-eval (cadr name) env))
-                        => write-line)
-                       (else (not-found 'documentation (cadr name)))))
+                 (let ((value (local-eval (cadr name) env)))
+                   (cond ((try-value-help (cadr name) value)
+                          => noop)
+                         ((object-documentation value)
+                          => write-line)
+                         (else (not-found 'documentation (cadr name))))))
 
                 ;; (quote SYMBOL)
                 ((and (list? name)
@@ -109,7 +164,8 @@ You don't seem to have regular expressions installed.\n"))
   (let ((entries (apropos-fold (lambda (module name object data)
 				 (cons (list module
 					     name
-					     (object-documentation object)
+					     (or (try-value-help name object)
+                                                 (object-documentation object))
 					     (cond ((closure? object)
 						    "a procedure")
 						   ((procedure? object)

-- 
http://wingolog.org/

Re: pushed to master: extensibility to (ice-9 session)

[Ludovic Courtès]

Hello,

Andy Wingo <wingo@pobox.com> writes:

> I pushed the following patch to master. Is it OK to push to 1.8 as well?
> That way I could drop some modules from guile-lib, and make guile-lib
> depend on guile >= 1.8.x.

Looks OK to me.  (Too bad this module isn't documented BTW.)  Do you
have example use cases?

> (Perhaps we can set up a list for patches that get pushed to Guile ?)

Yes!

A few remarks:

> +(define *value-help-handlers* '())

The convention within Guile is rather `%'-prefixed names for globals, as
in `%load-path'.

> +                 (let ((value (local-eval (cadr name) env)))
> +                   (cond ((try-value-help (cadr name) value)
> +                          => noop)
> +                         ((object-documentation value)
> +                          => write-line)
> +                         (else (not-found 'documentation (cadr name))))))

Shouldn't `object-documentation' as a default value helper, as in:

  (define %value-help-handlers
    `(,(lambda (n v) (object-documentation v))))

Thanks,
Ludo'.

Re: pushed to master: extensibility to (ice-9 session)

[Neil Jerram]

ludo@gnu.org (Ludovic Courtès) writes:

>> +(define *value-help-handlers* '())
>
> The convention within Guile is rather `%'-prefixed names for globals, as
> in `%load-path'.

I'm not sure about that.  I interpret `%' as something to do with the
"system" (e.g. when I was proposing %get-stack-depth).  I would agree
with the patch that *name* is the convention - if anything is - for
global variables.

Regards,
     Neil

Re: pushed to master: extensibility to (ice-9 session)

[Neil Jerram]

Andy Wingo <wingo@pobox.com> writes:

> Hi,
>
> I pushed the following patch to master. Is it OK to push to 1.8 as well?
> That way I could drop some modules from guile-lib, and make guile-lib
> depend on guile >= 1.8.x.

I have no objection to that.

> (Perhaps we can set up a list for patches that get pushed to Guile ?)

Not sure I understand...  Anything like guile-sources, which we
already have?

>    :use-module (ice-9 rdelim)
> -  :export (help apropos apropos-internal apropos-fold
> -	   apropos-fold-accessible apropos-fold-exported apropos-fold-all
> -	   source arity system-module))
> +  :export (help
> +           add-value-help-handler! remove-value-help-handler!
> +           add-name-help-handler! remove-name-help-handler!
> +           apropos apropos-internal apropos-fold apropos-fold-accessible
> +           apropos-fold-exported apropos-fold-all source arity
> +           system-module module-commentary))

Also say something in the commit about exporting module-commentary ?

> +`proc' will be called as (PROC NAME VALUE). `proc' should return #t to
> +indicate that it has performed help, a string to override the default
> +object documentation, or #f to try the other handlers, potentially
> +falling back on the normal behavior for `help'."

> +The return value of `proc' is as specified in
> +`add-value-help-handler!'."

I'd prefer to repeat what it said in add-value-help-handler!, so that
this doc stands alone.

(It would be quite ironic if a procedure for extending the `help'
system doesn't have completely useful `help' itself.)

> +  (set! *name-help-handlers* (cons proc *name-help-handlers*)))
> +
> +(define (remove-name-help-handler! proc)
> +  "Removes a handler for performing `help' on a name.
> +
> +See the documentation for `add-name-help-handler' for more
> +information."

What is the point of that last sentence?  I suspect nothing, so
recommend removing it.  (And same for remove-value-help-handler!)

> -                 (cond ((object-documentation
> -                         (local-eval (cadr name) env))
> -                        => write-line)
> -                       (else (not-found 'documentation (cadr name)))))
> +                 (let ((value (local-eval (cadr name) env)))
> +                   (cond ((try-value-help (cadr name) value)
> +                          => noop)
> +                         ((object-documentation value)
> +                          => write-line)
> +                         (else (not-found 'documentation (cadr name))))))

Why noop here?  Won't that discard the documentation?

Regards,
     Neil

Re: pushed to master: extensibility to (ice-9 session)

[Ludovic Courtès]

Hi Neil,

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

> ludo@gnu.org (Ludovic Courtès) writes:
>
>> The convention within Guile is rather `%'-prefixed names for globals, as
>> in `%load-path'.
>
> I'm not sure about that.  I interpret `%' as something to do with the
> "system" (e.g. when I was proposing %get-stack-depth).

Hmm, good point.

> I would agree with the patch that *name* is the convention - if
> anything is - for global variables.

Fine by me, then!

Ludo'.

Re: pushed to master: extensibility to (ice-9 session)

[Clinton Ebadi]

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

> ludo@gnu.org (Ludovic Courtès) writes:
>
>>> +(define *value-help-handlers* '())
>>
>> The convention within Guile is rather `%'-prefixed names for globals, as
>> in `%load-path'.
>
> I'm not sure about that.  I interpret `%' as something to do with the
> "system" (e.g. when I was proposing %get-stack-depth).  I would agree
> with the patch that *name* is the convention - if anything is - for
> global variables.

*FOO* to me implies that it is a fluid (as the *FOO* tradition comes
 from Common Lisp where it is used to mark dynamically scoped
 variables). Unadorned `value-help-handlers' seems more in line with
 Scheme style for a global lexical variable.

-- 
No, there's nothing here about X, so be quiet.

Re: pushed to master: extensibility to (ice-9 session)

[Ludovic Courtès]

Hi,

Clinton Ebadi <clinton@unknownlamer.org> writes:

> *FOO* to me implies that it is a fluid (as the *FOO* tradition comes
>  from Common Lisp where it is used to mark dynamically scoped
>  variables).

I had that feeling, too (although I never wrote a line of CL), and I
follow this convention in my Scheme projects, but I see that at least
Bigloo uses that convention for globals.  Scheme48 people seem to like
using `&'-prefixed names, as in SRFI-35 and parts of R6RS.  So Neil is
probably right in saying there's no convention.  ;-)

Ludo'.

Re: pushed to master: extensibility to (ice-9 session)

[Andy Wingo]

Hi,

On Tue 27 Jan 2009 21:30, ludo@gnu.org (Ludovic Courtès) writes:

> Looks OK to me. (Too bad [(ice-9 session)] isn't documented BTW.) Do
> you have example use cases?

From (texinfo reflection):

(cond
 ((defined? 'add-value-help-handler!)
  (define (stexi-help-handler name value)
    (stexi->plain-text (object-stexi-documentation value name #:force #t)))
  (define (module-help-handler name)
    (and (list? name)
         (and-map symbol? name)
         (stexi->plain-text (module-stexi-documentation name))))

  (add-value-help-handler! stexi-help-handler)
  (add-name-help-handler! module-help-handler)))

>> +(define *value-help-handlers* '())
>
> The convention within Guile is rather `%'-prefixed names for globals, as
> in `%load-path'.

I'm going to have to agree with Neil here ;) See also %make-void-port,
%search-load-path, %package-data-dir, etc. Search the manual for
scm_sys_ for more.

> Shouldn't `object-documentation' as a default value helper, as in:
>
>   (define %value-help-handlers
>     `(,(lambda (n v) (object-documentation v))))

Yes, nice one.

Thanks,

Andy
-- 
http://wingolog.org/

Re: pushed to master: extensibility to (ice-9 session)

[Andy Wingo]

Hi Neil,

On Tue 27 Jan 2009 23:10, Neil Jerram <neil@ossau.uklinux.net> writes:

> I have no objection to that.
>
>> (Perhaps we can set up a list for patches that get pushed to Guile ?)
>
> Not sure I understand...  Anything like guile-sources, which we
> already have?

I mean to say, a list that gets a mail whenever something is pushed to
master, automatically -- for reviewing each other's commits, and knowing
what's going on. (I appreciate the detailed review :)

>>    :use-module (ice-9 rdelim)
>> -  :export (help apropos apropos-internal apropos-fold
>> -	   apropos-fold-accessible apropos-fold-exported apropos-fold-all
>> -	   source arity system-module))
>> +  :export (help
>> +           add-value-help-handler! remove-value-help-handler!
>> +           add-name-help-handler! remove-name-help-handler!
>> +           apropos apropos-internal apropos-fold apropos-fold-accessible
>> +           apropos-fold-exported apropos-fold-all source arity
>> +           system-module module-commentary))
>
> Also say something in the commit about exporting module-commentary ?

Ah, that would have been good, yes. (Of course even better would be
documentation, but I don't have time to fix that one right now.)

>> +The return value of `proc' is as specified in
>> +`add-value-help-handler!'."
>
> I'd prefer to repeat what it said in add-value-help-handler!, so that
> this doc stands alone.

Ok, done.

>> +See the documentation for `add-name-help-handler' for more
>> +information."
>
> What is the point of that last sentence?  I suspect nothing, so
> recommend removing it.  (And same for remove-value-help-handler!)

Done

>> +                   (cond ((try-value-help (cadr name) value)
>> +                          => noop)
>
> Why noop here?  Won't that discard the documentation?

    `noop' is a primitive procedure in the (guile) module.

     -- Scheme Procedure: noop . args
         Do nothing.  When called without arguments, return `#f', otherwise
         return the first argument.

Agreed that it's unclear, I'll change to be more idiomatic.

Andy
-- 
http://wingolog.org/

Re: pushed to master: extensibility to (ice-9 session)

[Ludovic Courtès]

Hi,

Andy Wingo <wingo@pobox.com> writes:

> On Tue 27 Jan 2009 21:30, ludo@gnu.org (Ludovic Courtès) writes:
>
>> Looks OK to me. (Too bad [(ice-9 session)] isn't documented BTW.) Do
>> you have example use cases?
>
> From (texinfo reflection):
>
> (cond
>  ((defined? 'add-value-help-handler!)
>   (define (stexi-help-handler name value)
>     (stexi->plain-text (object-stexi-documentation value name #:force #t)))
>   (define (module-help-handler name)
>     (and (list? name)
>          (and-map symbol? name)
>          (stexi->plain-text (module-stexi-documentation name))))
>
>   (add-value-help-handler! stexi-help-handler)
>   (add-name-help-handler! module-help-handler)))

That's really nice!

I think we should aim for the integration of SSAX and STexi in Guile
2.0.  The latter would allow us to do nice things regarding with
automatically extracted documentation.

Thanks,
Ludo'.

Re: pushed to master: extensibility to (ice-9 session)

[Andy Wingo]

Hello!

On Wed 28 Jan 2009 13:26, ludo@gnu.org (Ludovic Courtès) writes:

> I think we should aim for the integration of SSAX and STexi in Guile
> 2.0.  The latter would allow us to do nice things regarding with
> automatically extracted documentation.

Sure, would be a nice idea. I don't think it's a blocker though -- it's
something that can be added after a 2.0 release. It would be a bit of
work to get it documented properly, I think, and personally I'd like to
concentrate on some other code. But I'll help anyone that's interested
in doing it :)

Cheers,

Andy
-- 
http://wingolog.org/

Re: pushed to master: extensibility to (ice-9 session)

[Neil Jerram]

Andy Wingo <wingo@pobox.com> writes:

> Hi Neil,

Hi Andy...

> On Tue 27 Jan 2009 23:10, Neil Jerram <neil@ossau.uklinux.net> writes:
>
>> Not sure I understand...  Anything like guile-sources, which we
>> already have?
>
> I mean to say, a list that gets a mail whenever something is pushed to
> master, automatically -- for reviewing each other's commits, and knowing
> what's going on. (I appreciate the detailed review :)

I'm not sure that's important.  It seems easy to me to use the web UI
at savannah to keep an eye on what's happening, and to cut and paste
commitdiffs from there for review.  But I don't object if you and
Ludovic think this would be useful.

>> Why noop here?  Won't that discard the documentation?
>
>     `noop' is a primitive procedure in the (guile) module.
>
>      -- Scheme Procedure: noop . args
>          Do nothing.  When called without arguments, return `#f', otherwise
>          return the first argument.
>
> Agreed that it's unclear, I'll change to be more idiomatic.

I think `identity' has the same effect, and to me that's a clearer
name in this context; so you could use that.

Finally, there's no NEWS entry in your commit - should there be one?
(Unless we're planning some kind of automatic scan for new module
exports, for when we do the 2.0 release, I think there should be some
NEWS here.)

Regards,
        Neil

Re: pushed to master: extensibility to (ice-9 session)

[szgyg]

Andy Wingo wrote:
> 
> I mean to say, a list that gets a mail whenever something is pushed to
> master, automatically -- for reviewing each other's commits, and knowing
> what's going on. (I appreciate the detailed review :)

Savannah provides atom feeds, like
http://git.savannah.gnu.org/cgit/guile.git/atom/?h=master

s