bug#29684: exception printers - request for improvement

bug#29684: exception printers - request for improvement

[David Pirotte]

[-- Attachment #1.1: Type: text/plain, Size: 1176 bytes --]

Hello,

	The attached patched is from Daniel Lloren, I'm just 'a messenger' (and added
	a comment in the source, preceding the new binding).

The proposed patch is to allow  exception printers user customization.

This has been very important, not to say vital, for those of us who manipulate large
structures, lists, arrays, sfri-4 bytevectors, ..., something we have been doing
locally ... but we need something for our users (aiscm, guile-cv ...), so they don't
have to patch guile locally... (most would be scared to do so and would not do it
anyway...).

Once applied, users can, for example, customize the raised exception system so it
uses truncated-print, either individually (in .guile), or guile admins can do
this globally (in share/guile-site/init.scm):

	(use-modules (ice-9 pretty-print))
                    
	(when (defined? 'exception-format)
	  (set! exception-format
	        (lambda (port fmt . args)
	          (for-each (lambda (arg)
	                      (truncated-print arg #:port port))
	              args))))

Maybe there is another/better approach, I don't know, but this works pretty well
for me...

Thanks,
David

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: 0002-Allowing-exception-printers-user-customization.patch --]
[-- Type: text/x-patch, Size: 7011 bytes --]

From 772cc05b1fe481a43be4c17c90ed3788cf37d2a6 Mon Sep 17 00:00:00 2001
From: David Pirotte <david@altosw.be>
Date: Wed, 13 Dec 2017 00:43:30 -0200
Subject: [PATCH 2/2] Allowing exception printers user customization

* module/ice-9/boot-9.scm (exception-format, dispatch-exception,
  exception-printers, scm-error-printer, syntax-error-printer,
  keyword-error-printer, getaddrinfo-error-printer, false-if-exception,
  make-record-type):  Instead of using 'format', let's define a specific
  format binding for exception printers, to allow its user
  customization.
---
 module/ice-9/boot-9.scm | 46 ++++++++++++++++++++++++++--------------------
 1 file changed, 26 insertions(+), 20 deletions(-)

diff --git a/module/ice-9/boot-9.scm b/module/ice-9/boot-9.scm
index 751a3bcd1..cbbedac15 100644
--- a/module/ice-9/boot-9.scm
+++ b/module/ice-9/boot-9.scm
@@ -326,6 +326,10 @@ If returning early, return the return value of F."
 
 (define format simple-format)
 
+;; instead of using the above, let's define a specific format binding
+;; for exception printers, to allow its user customization.
+(define exception-format simple-format)
+
 ;; this is scheme wrapping the C code so the final pred call is a tail call,
 ;; per SRFI-13 spec
 (define string-any
@@ -762,7 +766,7 @@ information is unavailable."
                        ((not (car args)) 1)
                        (else 0))))
      (else
-      (format (current-error-port) "guile: uncaught throw to ~a: ~a\n"
+      (exception-format (current-error-port) "guile: uncaught throw to ~a: ~a\n"
               key args)
       (primitive-exit 1))))
 
@@ -865,8 +869,8 @@ for key @var{k}, then invoke @var{thunk}."
           (let ((filename (or (cadr source) "<unnamed port>"))
                 (line (caddr source))
                 (col (cdddr source)))
-            (format port "~a:~a:~a: " filename (1+ line) col))
-          (format port "ERROR: "))))
+            (exception-format port "~a:~a:~a: " filename (1+ line) col))
+          (exception-format port "ERROR: "))))
 
   (set! set-exception-printer!
         (lambda (key proc)
@@ -875,7 +879,7 @@ for key @var{k}, then invoke @var{thunk}."
   (set! print-exception
         (lambda (port frame key args)
           (define (default-printer)
-            (format port "Throw to key `~a' with args `~s'." key args))
+            (exception-format port "Throw to key `~a' with args `~s'." key args))
 
           (when frame
             (print-location frame port)
@@ -884,7 +888,7 @@ for key @var{k}, then invoke @var{thunk}."
                           (lambda () (frame-procedure-name frame))
                           (lambda _ #f))))
               (when name
-                (format port "In procedure ~a:\n" name))))
+                (exception-format port "In procedure ~a:\n" name))))
 
           (catch #t
             (lambda ()
@@ -893,7 +897,9 @@ for key @var{k}, then invoke @var{thunk}."
                     (printer port key args default-printer)
                     (default-printer))))
             (lambda (k . args)
-              (format port "Error while printing exception.")))
+              (exception-format
+               port "Error while printing exception `~a`: `~a' with args [~s]"
+               key k args)))
           (newline port)
           (force-output port))))
 
@@ -907,38 +913,38 @@ for key @var{k}, then invoke @var{thunk}."
     (apply (case-lambda
              ((subr msg args . rest)
               (if subr
-                  (format port "In procedure ~a: " subr))
-              (apply format port msg (or args '())))
+                  (exception-format port "In procedure ~a: " subr))
+              (apply exception-format port msg (or args '())))
              (_ (default-printer)))
            args))
 
   (define (syntax-error-printer port key args default-printer)
     (apply (case-lambda
              ((who what where form subform . extra)
-              (format port "Syntax error:\n")
+              (exception-format port "Syntax error:\n")
               (if where
                   (let ((file (or (assq-ref where 'filename) "unknown file"))
                         (line (and=> (assq-ref where 'line) 1+))
                         (col (assq-ref where 'column)))
-                    (format port "~a:~a:~a: " file line col))
-                  (format port "unknown location: "))
+                    (exception-format port "~a:~a:~a: " file line col))
+                  (exception-format port "unknown location: "))
               (if who
-                  (format port "~a: " who))
-              (format port "~a" what)
+                  (exception-format port "~a: " who))
+              (exception-format port "~a" what)
               (if subform
-                  (format port " in subform ~s of ~s" subform form)
+                  (exception-format port " in subform ~s of ~s" subform form)
                   (if form
-                      (format port " in form ~s" form))))
+                      (exception-format port " in form ~s" form))))
              (_ (default-printer)))
            args))
 
   (define (keyword-error-printer port key args default-printer)
     (let ((message (cadr args))
           (faulty  (car (cadddr args)))) ; I won't do it again, I promise.
-      (format port "~a: ~s" message faulty)))
+      (exception-format port "~a: ~s" message faulty)))
 
   (define (getaddrinfo-error-printer port key args default-printer)
-    (format port "In procedure getaddrinfo: ~a" (gai-strerror (car args))))
+    (exception-format port "In procedure getaddrinfo: ~a" (gai-strerror (car args))))
 
   (set-exception-printer! 'goops-error scm-error-printer)
   (set-exception-printer! 'host-not-found scm-error-printer)
@@ -1066,11 +1072,11 @@ VALUE."
        (lambda (key . args)
          (for-each (lambda (s)
                      (if (not (string-null? s))
-                         (format (current-warning-port) ";;; ~a\n" s)))
+                         (exception-format (current-warning-port) ";;; ~a\n" s)))
                    (string-split
                     (call-with-output-string
                      (lambda (port)
-                       (format port template arg ...)
+                       (exception-format port template arg ...)
                        (print-exception port #f key args)))
                     #\newline))
          #f)))))
@@ -1229,7 +1235,7 @@ VALUE."
                 (if (= (length args) nfields)
                     (apply make-struct/no-tail rtd args)
                     (scm-error 'wrong-number-of-args
-                               (format #f "make-~a" type-name)
+                               (exception-format #f "make-~a" type-name)
                                "Wrong number of arguments" '() #f)))))))))
 
   (define (default-record-printer s p)
-- 
2.15.1


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

bug#29684: exception printers - request for improvement

[Daniel Llorens]

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


Agreed with David that this is an important issue. Without a patch, working with a large data structure is guaranteed to kill the REPL session sooner or later. There was a thread a while ago here: https://lists.gnu.org/archive/html/guile-user/2017-02/msg00188.html

Right now, the REPL will catch either general exceptions or calls to error or scm-error which throw specific exception types. For general exceptions the argument list is just printed with ~a, but for libguile exceptions, the handler expects a ‘format’ argument which is used to format the error message. One can put ~a and ~s in this ‘format’ argument, see https://www.gnu.org/software/guile/manual/html_node/Handling-Errors.html.

The proposal

> 	(use-modules (ice-9 pretty-print))
> 
> 	(when (defined? 'exception-format)
> 	  (set! exception-format
> 	        (lambda (port fmt . args)
> 	          (for-each (lambda (arg)
> 	                      (truncated-print arg #:port port))
> 	              args))))

throws away this formatting. That's why I proposed instead the other solution in the links above, which replaces the ~a and ~s by ~y (this calls truncated print), but otherwise respects the original format string.

(set! exception-format
         (lambda (port fmt . args)
           (apply format port (rewrite-fmt (rewrite-fmt fmt "~s") "~a") args)))

Anyway. These are clunky hacks. According to the manual

> ~A indicates an argument printed using display, while ~S indicates an argument printed using write


I think this is an unnecessary complication if there's a hook. One option (that works like ‘write’ by default) is enough. If one wants to craft a special error message, that should be done without abusing the exception arguments.

I think this ‘write’ should be a hook or even a fluid. There is already a hook to configure the REPL printer, which is set by (repl-default-option-set! 'print ##) or (repl-option-set! repl 'print ##) (the latter is undocumented). Maybe that hook can be reused for the exception printer. Or maybe there can be a new hook that is reused by both the repl and the exception printer. It makes sense to me to make both of these refer to the same function.

I also agree with David that both the REPL and the exception printers should be truncated by default. That requires truncated-print to be available in (ice-9 boot).

Anyway, just some thoughts.

See the related bug#29669 about REPL printers. 

Regards

	Daniel

[-- Attachment #2: Type: text/html, Size: 4465 bytes --]

bug#29684: exception printers - request for improvement

[Ludovic Courtès]

Hi David,

David Pirotte <david@altosw.be> skribis:

> +;; instead of using the above, let's define a specific format binding
> +;; for exception printers, to allow its user customization.
> +(define exception-format simple-format)
> +
>  ;; this is scheme wrapping the C code so the final pred call is a tail call,
>  ;; per SRFI-13 spec
>  (define string-any
> @@ -762,7 +766,7 @@ information is unavailable."
>                         ((not (car args)) 1)
>                         (else 0))))
>       (else
> -      (format (current-error-port) "guile: uncaught throw to ~a: ~a\n"
> +      (exception-format (current-error-port) "guile: uncaught throw to ~a: ~a\n"

It seems to me that you can achieve similar effect, at least in some
cases, by parameterizing ‘current-error-port’.  It also makes more sense
to me to parameterize ‘current-error-port’ rather than set
‘exception-format’, because the ‘exception-format’ is very constrained:
it has to implement at least everything ‘simple-format’ implements.

WDYT?

Besides, note that there’s also ‘set-exception-printer!’.  It’s a
different kind of customization, but it can serve a similar purpose.

Thanks,
Ludo’.

bug#29684: exception printers - request for improvement

[David Pirotte]

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

Hello Ludovic,

> ...
> It seems to me that you can achieve similar effect, at least in some
> cases, by parameterizing ‘current-error-port’.  It also makes more sense
> to me to parameterize ‘current-error-port’ rather than set
> ‘exception-format’, because the ‘exception-format’ is very constrained:
> it has to implement at least everything ‘simple-format’ implements.
> 
> WDYT?
> 
> Besides, note that there’s also ‘set-exception-printer!’.  It’s a
> different kind of customization, but it can serve a similar purpose.

To be honest, exactly how does not really matter, I leave that to you (maintainers):
I have used the code, actually written by Daniel, who also claimed it was not
complete, iirc, to raise the issue, so we can discuss it between us:

	I think we should be very (very) friendly to new Guile users, and especially
	those who do not know scheme (yet);

	I think I should be able to use Guile-CV to teach basic image manipulation
	to a class of young teenagers, who would not even know what scheme is, which
	means, imo, no guile customization should be required:

	guile
	(use-modules (cv))
	(im-load "my-preferred-penguin.png")

	- at this point, even if the image is very small, guile is 'lost' [*]
	- same if I teach them to compose a few image ops, they commit
	a mistake while typing .. bang, the exception printer is 'lost'

It is perfectly fine, on the opposite side, to ask advanced scheme users (who would
want to, but I really don't see the point) configure guile s the repl and the
exception printer so it tries to print the full content of huge lsts, vectors, arrays
...

David

[*]	one could argue i could have chosen another data structure, but well, I
	did not :), and imo, we can't tell users who have to manipulate large
	structures (arrays, vector, think about non s/w engineers scientists,
	biologists ...),	"... don't manipulate large structures in a repl, wrap them
	in a record first ..."

	it happened to me at university, trying to make a demo of Guile-CV to a group
	of postdoc candidates: I updated guile the day before and forgot to repatch
	both the repl and the exception printers, opened an image, and ... bang!

	I was in emacs, so ... after a few seconds they all laughed and ... by
	the time I could recover, repatched guile ... they were all ...
	gone! (and convinced they should stick to python and open-cv or java and
	imagej ...

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

bug#29684: exception printers - request for improvement

[Ludovic Courtès]

Hi David,

David Pirotte <david@altosw.be> skribis:

> It is perfectly fine, on the opposite side, to ask advanced scheme users (who would
> want to, but I really don't see the point) configure guile s the repl and the
> exception printer so it tries to print the full content of huge lsts, vectors, arrays
> ...

Perhaps you could arrange to set the REPL’s ‘print’ option right from
one of the Guile-OpenCV modules?  I mean the configuration you mention
have doesn’t have to be done manually by users.

Ludo’.

bug#29684: exception printers - request for improvement

[David Pirotte]

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

Hi Ludovic,

> > It is perfectly fine, on the opposite side, to ask advanced scheme users (who
> > would want to, but I really don't see the point) configure guile s the repl and
> > the exception printer so it tries to print the full content of huge lsts,
> > vectors, arrays ...  

> Perhaps you could arrange to set the REPL’s ‘print’ option right from
> one of the Guile-OpenCV modules?  I mean the configuration you mention
> have doesn’t have to be done manually by users.

I'll do that yes, since I failed to convince you it you be a better default for all
of us, not just Guile-CV :)

I hope I can also do that wrt exceptions in an easy way too, I did not look with care
to what you suggested, since I thought what Daniel offered was a better approach and
that it would be included one way or another in 2.2.4 ... let's see what I can cook
with set-exception-printer! then :)

Thanks,
David

Note that, for info, I named the project Guile-CV, not to refer to existing libs,
and currently, it does not use Open-CV :) - which doesn't have a maintained C
wrapper :(, but it would be nice to be able to use it 'from Guile-CV', ... one day
maybe ...

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

bug#29684: exception printers - request for improvement

[tomas]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On Tue, Jul 03, 2018 at 04:31:23PM -0300, David Pirotte wrote:
> Hi Ludovic,
> 
> > > It is perfectly fine, on the opposite side, to ask advanced scheme users (who
> > > would want to, but I really don't see the point) configure guile s the repl and
> > > the exception printer so it tries to print the full content of huge lsts,
> > > vectors, arrays ...  
> 
> > Perhaps you could arrange to set the REPL’s ‘print’ option right from
> > one of the Guile-OpenCV modules?  I mean the configuration you mention
> > have doesn’t have to be done manually by users.
> 
> I'll do that yes, since I failed to convince you it you be a better default for all
> of us, not just Guile-CV :)

Hm. Applying the peanut gallery factor to my opinion (roughly 10e-7), I tend to
side with David's position: the default consumer of REPL's print is a human, and
feeding him/her with huge binary things isn't "tasty"...

But... grain of salt, and things. What would be David's position's downsides?

Thanks
- -- tomás
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.12 (GNU/Linux)

iEYEARECAAYFAls8k6QACgkQBcgs9XrR2kaJSgCePeZgTTphZmdrkww/uoVm/9mW
Ib4An2/NY2kWS6Oso7w9jzSP8s9x9mZz
=KsrJ
-----END PGP SIGNATURE-----

bug#29684: exception printers - request for improvement

[David Pirotte]

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

Hi Thomas,

> ...
> Hm. Applying the peanut gallery factor to my opinion (roughly 10e-7), I tend to
> side with David's position: the default consumer of REPL's print is a human, and
> feeding him/her with huge binary things isn't "tasty"...

> But... grain of salt, and things. What would be David's position's downsides?

If the change I'm proposing is implemented in such a way that it still offer,
through an option mechanism, to display several lines ... (and it has to be
implemented this way, as also spotted by Andy in a subsequent email), then
I fail to see any downsides.

Regards,
David

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]