[Maxima-commits] [git] Maxima CAS branch, master, updated. branch-5_31-base-183-gf44d669

Leo Butler <l_butler at users.sourceforge.net> writes:
>>   > I intentionally didn't try to use *prompt-prefix* and *prompt-suffix*
>>   > here because I know that they are designed for front-ends to "tag" their
>>   > prompts easily. Since they are just strings, I couldn't think of a way
>>   > to add a note after the prompt without potentially clobbering them. But
>>   > maybe I missed something?
>>
>>   I dunno, I'm not convinced the prompt note system is useful enough.
>>   I can see that being able to rework the prompts is going to be useful
>>   for people writing user interfaces -- agreed on that point. But isn't
>>   it enough for that purpose to concentrate the prompt formatting into
>>   a single function (as in the previous commit, which, as I said, is OK
>>   by me) which can then be replaced easily enough? We could even make the
>>   prompt formatter a Maxima function so that it can be replaced by Maxima
>>   code.
>
> Attached is a patch that includes Robert's suggestion. I change
> format-prompt so that it mfuncall-s $format_prompt if this is non-nil.

Leo: Great, thanks for sending a patch. It would have been helpful if
you'd answered some of my questions, as well / instead. I still don't
really know what you think about some of the things I raised.

I'm going to put comments inline, but the conclusion: This patch removes
my code (fine), adds a broken replacement and also adds an interface for
Maxima code to customise *alt-display1d* and *alt-display2d*. I believe
that this last part is unrelated to my changes under discussion.

I don't believe that this patch is a better fit for Maxima core than
what's currently there. I agree that it could be tidied up to make an
alternative. Also, I don't believe that it is simpler than what I wrote!

Robert: Am I right in thinking you want to make a release soon? In that
case, how about I revert my patches for now and we can discuss this
further post-release?

Comments inline:

> ===File ~/git.diff==========================================
> diff --git a/doc/info/Command.texi b/doc/info/Command.texi
> index 24eac12..fb0afcf 100644
> --- a/doc/info/Command.texi
> +++ b/doc/info/Command.texi
> @@ -728,28 +728,6 @@ Default value: @code{_}
>  @end defvr
>  
>  @c -----------------------------------------------------------------------------
> - at deffn {Function} prompt_prepend_note (note)
> -
> -Add a note that is displayed after every input prompt. This note must
> -either be a string or it should be something that can be called as a
> -function. In the latter case, the function should return either a
> -string or a number (for technical implementation reasons, arbitrary
> -expressions might not be printed correctly). If the function call
> -raises an error then the note is removed from the list to be
> -displayed from then on.
> -
> -There can be more than one note displayed after a prompt, and this
> -adds the given note to the left hand end of the list. To remove a note
> -from the list, use @mref{prompt_remove_note}.
> - at end deffn
> -
> - at deffn {Function} prompt_remove_note (note)
> -
> -Remove a note from the list of objects to display after a prompt. See
> - at mref{prompt_prepend_note} for how to add one.
> - at end deffn
> -
> - at c -----------------------------------------------------------------------------
>  @anchor{quit}
>  @deffn {Function} quit ()

Where is the documentation for the function you add?

>  
> diff --git a/share/contrib/alt-display/alt-display.lisp b/share/contrib/alt-display/alt-display.lisp
> new file mode 100644
> index 0000000..80a9ccb
> --- /dev/null
> +++ b/share/contrib/alt-display/alt-display.lisp
> @@ -0,0 +1,43 @@
> +;; -*- mode: lisp -*-
> +;; Copyright Leo Butler (l_butler at users.sourceforge.net) 2013
> +;; Released under the terms of GPLv2+
> +
> +(defun $set_prompt (type value &rest args)
> +  (declare (special *prompt-prefix* *prompt-suffix*))
> +  (assert (and (symbolp type) (or (member type '($s $su $suf $suff $suffi $suffix))
> +				  (member type '($p $pr $pre $pref $prefi $prefix)))

What is the use case for this? "prefix" isn't really that long a word...

> +	       (or (stringp value) (null value)))
> +	  (type value)
> +	  "set_prompt(type, value): type must be either 'prefix or 'suffix, value must be a string or false.~%type=~a, value=~a" type value)
> +  (ecase type
> +    (($p $pr $pre $pref $prefi $prefix) (setq *prompt-prefix* (or value "")))
> +    (($s $su $suf $suff $suffi $suffix) (setq *prompt-suffix* (or value "")))
> +    )
> +  (if args (apply '$set_prompt args))
> +  '$done)
> +
> +(defun $set_alt_display (type f &rest args)
> +  (assert (and (member type '(1 2))
> +	       t)
> +	  (type f)
> +	  "set_alt_display(type,f): type must equal 1 or 2, f must be a function.")
> +  (let ((alt-display (ecase type
> +		       (1 '*alt-display1d*)
> +		       (2 '*alt-display2d*))))

I really dislike this, using numbers to choose between options. (Even if
the options are 1d and 2d). Can't we just choose which display to set
based on $display2d? So the user could write

block([display2d: true]
  set_alt_display (display_2d),
  display2d: false,
  set_alt_display (display_1d))$

or whatever.

Also, do you intend to throw a lisp error if someone types
"set_alt_display(3,"boo");" ?

> +    (flet ((reset-display (&optional no-warn)
> +	     (unless no-warn (warn "Resetting display."))
> +	     (set alt-display nil)))
> +      (cond ((null f) (reset-display t))
> +	    (t
> +	     (handler-case
> +		 (set alt-display
> +		      (coerce (lambda(form)
> +				(handler-case (mfuncall f form)
> +				  (error () "Error in ~a. ~a" alt-display (reset-display))))
> +			      'function))
> +	       (error (msg) "In ~a, an error has occurred, resetting to ~a.~%~a" alt-display (reset-display) msg))))))
> +  (if args (apply #'$set_alt_display args))
> +  '$done)
> +    
> +
> +; end of alt-display.lisp 
> diff --git a/share/contrib/alt-display/alt-display.mac b/share/contrib/alt-display/alt-display.mac
> new file mode 100644
> index 0000000..ee259b5
> --- /dev/null
> +++ b/share/contrib/alt-display/alt-display.mac
> @@ -0,0 +1,22 @@
> +/* -*- Mode: maxima; Package: MAXIMA -*- */
> +/*
> +;; Copyright Leo Butler (l_butler at users.sourceforge.net) 2013
> +;; Released under the terms of GPLv2+
> +*/
> +
> +load("alt-display.lisp");
> +
> +define_alt_display(f,body) ::= buildq(
> +  [f:f,
> +  body:psubst([
> +    alt_display1d='?\*alt\-display1d\*,
> +    alt_display2d='?\*alt\-display2d\*,
> +    prompt_prefix='?\*prompt\-prefix\*,
> +    prompt_suffix='?\*prompt\-suffix\*,
> +    displa='?displa
> +    ],
> +    body)],
> +  f := block([simp:false], body));
> +    
> +
> +/* end of alt-display.mac */

<snip: mostly deletions>

>  (defun format-prompt (destination control-string &rest arguments)
>    "Like AFORMAT, but add the prefix and suffix configured for a prompt. This
>  function deals correctly with the ~M control character, but only when
> -DESTINATION is an actual stream (rather than nil for a string).
> -
> -This function also obeys the special variable *DISPLAYING-INPUT-PROMPT*. If that
> -is true, it also shows prompt notes, as set with prompt_prepend_note()."
> -  (let ((*print-circle* nil))
> -    (concatenate 'string
> -                 *prompt-prefix*
> -                 (apply 'aformat destination control-string arguments)
> -                 (if *displaying-input-prompt*
> -                     (prompt-notes)
> -                     "")
> -                 *prompt-suffix*)))
> +DESTINATION is an actual stream (rather than nil for a string)."
> +  (cond ($format_prompt
> +	 (mfuncall $format_prompt destination control-string arguments))
> +	(t
> +	 (let ((*print-circle* nil))
> +	   (concatenate 'string
> +			*prompt-prefix*
> +			(apply 'aformat destination control-string arguments)
> +		        *prompt-suffix*)))))

NO! This is a terrible idea. Please see my previous emails.

>  ;;  "When time began" (or at least the start of version control history),
>  ;;  the following comment was made at this point:
> @@ -112,12 +59,11 @@ is true, it also shows prompt notes, as set with prompt_prepend_note()."
>  ;;  don't deal correctly with ~M plus a string output stream.
>  (defun main-prompt ()
>    (declare (special *display-labels-p*))
> -  (let ((*displaying-input-prompt* t))
> -    (if *display-labels-p*
> -        (format-prompt nil "(~A~A) "
> -                       (print-invert-case (stripdollar $inchar))
> -                       $linenum)
> -        "")))
> +  (if *display-labels-p*
> +      (format-prompt nil "(~A~A) "
> +                     (print-invert-case (stripdollar $inchar))
> +                     $linenum)
> +      ""))
>  
>  (defun break-prompt ()
>    (format-prompt nil "~A"
> diff --git a/src/suprv1.lisp b/src/suprv1.lisp
> index 1d07a03..b08d495 100644
> --- a/src/suprv1.lisp
> +++ b/src/suprv1.lisp
> @@ -337,8 +337,7 @@
>  		    checkfactors nil greatorder nil lessorder nil $gensumnum 0
>  		    $weightlevels '((mlist)) *ratweights nil $ratweights
>  		    '((mlist simp))
> -		    tellratlist nil $dontfactor '((mlist)) $setcheck nil


Don't you want kill(all) to remove prompt and display customisations?


> -                    *prompt-notes* nil)
> +		    tellratlist nil $dontfactor '((mlist)) $setcheck nil)
>  	      (killallcontexts))
>  	     ((setq z (assoc x '(($inlabels . $inchar) ($outlabels . $outchar) ($linelabels . $linechar)) :test #'eq))
>  	      (mapc #'(lambda (y) (remvalue y '$kill))
> ============================================================
>
> ===File ~/git.log===========================================
> commit 117a2f3a6601e30f3d3766cd401f976467d8eade
> Author: Leo Butler <l_butler at users.sourceforge.net>
> Date:   Wed Dec 11 12:25:10 2013 -0500
>
>     contrib code to provide easy-to-use access to the $format_prompt hook
>     provided in the previous commit.
<snip>

Shouldn't this documentation be somewhere other than the git log?
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 315 bytes
Desc: not available
URL: <http://www.math.utexas.edu/pipermail/maxima/attachments/20131211/656ebc7f/attachment-0001.pgp>;