ewoc--adjust ugliness

ewoc--adjust ugliness

[Richard Stallman]

ewoc--adjust refers to a free variable `dll'.  This is not clean, because
that name is short and has no defvar.

When you want to bind a variable in one function
and use it in others, please give it a longer name,
starting with the package prefix, and give it a defvar.

Re: ewoc--adjust ugliness

[Thien-Thi Nguyen]

Richard Stallman <rms@gnu.org> writes:

> ewoc--adjust refers to a free variable `dll'.  This is not
> clean, because that name is short and has no defvar.
> 
> When you want to bind a variable in one function
> and use it in others, please give it a longer name,
> starting with the package prefix, and give it a defvar.

thanks for pointing this out.  i have installed a fix.

thi

Re: ewoc--adjust ugliness

[Mathias Dahl]

> > ewoc--adjust refers to a free variable `dll'.  This is not
> > clean, because that name is short and has no defvar.
>
> thanks for pointing this out.  i have installed a fix.

There seems to be quite a few things happening with ewoc right now,
has anyone looked into the recent bug in ewoc that Magnus found? See
below:

--

From: Magnus Henoch <mange@freemail.hu>
Subject: ewoc misbehaves
Newsgroups: gmane.emacs.pretest.bugs
Date: Sun, 21 May 2006 16:14:00 +0200

Revision 1.26 of ewoc.el changes its behavior in
backwards-incompatible ways.  Specifically:

- A newline is no longer inserted between items, and after header and
  footer.

- Markers after the ewoc structure are not advanced, but suddenly
  point before it after an item is inserted.

Magnus

--

No one has responded, as far as I can see, but maybe someone is
working on it? I for one cannot use the latest CVS Emacs because of
this as it breaks the nice jabber.el package.

Sorry if this seems like nagging (it is, kind of... :).

Re: ewoc--adjust ugliness

[Thien-Thi Nguyen]

   From: "Mathias Dahl" <mathias.dahl@gmail.com>
   Date: Fri, 26 May 2006 12:49:59 +0200

   No one has responded, as far as I can see, but maybe someone is
   working on it? I for one cannot use the latest CVS Emacs because of
   this as it breaks the nice jabber.el package.

   Sorry if this seems like nagging (it is, kind of... :).

there is some off-list discussion.  concensus is that the changes are ok;
now we are wondering how client packages (such as jabber.el) should best
go about detecting the change.  i suppose adding something like:

(put 'ewoc-create 'no-gratuitous-newline t)

would work, but am not sure how it fits in w/ emacs' conventions (if
any) for support of package-level features.  if no one objects, i will
add that line and document the discovery method.

in any case, there is an entry in NEWS for the changes under
"Incompatible Lisp Changes in Emacs 22.1", q.v.

thi

Re: ewoc--adjust ugliness

[Nick Roberts]

 > there is some off-list discussion.  concensus is that the changes are ok;

Do you work for the White House by any chance?


-- 
Nick                                           http://www.inet.net.nz/~nickrob

Re: ewoc--adjust ugliness

[Thien-Thi Nguyen]

   From: Nick Roberts <nickrob@snap.net.nz>
   Date: Sat, 27 May 2006 00:08:46 +1200

   Do you work for the White House by any chance?

like all political questions and answers,
that probably depends on who you ask...

i do work for GNU, but i do not work for anyone presently.  (if anyone
wants to discuss job offers / sponsorship, please use private mail.)

thi

Re: ewoc--adjust ugliness

[Stefan Monnier]

> go about detecting the change.  i suppose adding something like:
> (put 'ewoc-create 'no-gratuitous-newline t)
> would work, but am not sure how it fits in w/ emacs' conventions (if

I think it's more important to make it backward compatible, than to
allow clients to test easily whether the new feature is present.
First things first.


        Stefan

Re: ewoc--adjust ugliness

[Thien-Thi Nguyen]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> I think it's more important to make it backward compatible, than to
> allow clients to test easily whether the new feature is present.
> First things first.

i disagree (looks like i was too hasty concluding there was a
concensus), but more out of laziness than any contrarian sentiment.
hmm, writing that last sentence has shamed me into exerting a little
more effort [munge munge]...

please find below another patch.  i have tested it lightly w/ pcvs.el
and pcvs-info.el prior to 2006-05-18, and also w/ current sources,
modified to specify NOSEP in the call to `ewoc-create'.

 * emacs-lisp/ewoc.el (ewoc): Add member `hf-pp' to this structure.
 (ewoc--refresh-node): Reseat node's start marker.
 (ewoc--wrap): New func.
 (ewoc-create): Take additional arg NOSEP.  If nil, wrap node and
 header/footer pretty-printers.  Save header/footer pretty-printer.
 (ewoc-set-hf): Use ewoc's header/footer pretty-printer.

thanks go to Stefan for suggesting NOSEP.

thi

_______________________________________________________
diff -c -r1.30 ewoc.el
*** ewoc.el	26 May 2006 08:29:13 -0000	1.30
--- ewoc.el	26 May 2006 22:33:06 -0000
***************
*** 186,192 ****
  	    (:constructor ewoc--create
  			  (buffer pretty-printer header footer dll))
  	    (:conc-name ewoc--))
!   buffer pretty-printer header footer dll last-node)
  
  (defmacro ewoc--set-buffer-bind-dll-let* (ewoc varlist &rest forms)
    "Execute FORMS with ewoc--buffer selected as current buffer,
--- 186,192 ----
  	    (:constructor ewoc--create
  			  (buffer pretty-printer header footer dll))
  	    (:conc-name ewoc--))
!   buffer pretty-printer header footer dll last-node hf-pp)
  
  (defmacro ewoc--set-buffer-bind-dll-let* (ewoc varlist &rest forms)
    "Execute FORMS with ewoc--buffer selected as current buffer,
***************
*** 248,268 ****
  
  (defun ewoc--refresh-node (pp node)
    "Redisplay the element represented by NODE using the pretty-printer PP."
!   (let ((inhibit-read-only t)
!         (m (ewoc--node-start-marker node))
!         (R (ewoc--node-right node)))
      ;; First, remove the string from the buffer:
      (delete-region m (ewoc--node-start-marker R))
      ;; Calculate and insert the string.
      (goto-char m)
      (funcall pp (ewoc--node-data node))
      (ewoc--adjust m (point) R)))
  \f
  ;;; ===========================================================================
  ;;;                  Public members of the Ewoc package
  
  ;;;###autoload
! (defun ewoc-create (pretty-printer &optional header footer)
    "Create an empty ewoc.
  
  The ewoc will be inserted in the current buffer at the current position.
--- 248,281 ----
  
  (defun ewoc--refresh-node (pp node)
    "Redisplay the element represented by NODE using the pretty-printer PP."
!   (let* ((inhibit-read-only t)
!          (m (ewoc--node-start-marker node))
!          (pos (marker-position m))
!          (R (ewoc--node-right node)))
      ;; First, remove the string from the buffer:
      (delete-region m (ewoc--node-start-marker R))
      ;; Calculate and insert the string.
      (goto-char m)
      (funcall pp (ewoc--node-data node))
+     ;; If the ewoc was created w/ NOSEP, then this node's start marker will
+     ;; have been pushed back by the wrapped pretty printer (sigh).  Reseat it.
+     (set-marker m pos)
      (ewoc--adjust m (point) R)))
+ 
+ (defun ewoc--wrap (func)
+   (lexical-let ((ewoc--user-pp func))
+     (lambda (data)
+       (insert-before-markers "\n")
+       (forward-char -1)
+       (funcall ewoc--user-pp data)
+       (forward-char 1))))
+ 
  \f
  ;;; ===========================================================================
  ;;;                  Public members of the Ewoc package
  
  ;;;###autoload
! (defun ewoc-create (pretty-printer &optional header footer nosep)
    "Create an empty ewoc.
  
  The ewoc will be inserted in the current buffer at the current position.
***************
*** 275,297 ****
  
  Optional second and third arguments HEADER and FOOTER are strings,
  possibly empty, that will always be present at the top and bottom,
! respectively, of the ewoc."
    (let* ((dummy-node (ewoc--node-create 'DL-LIST 'DL-LIST))
           (dll (progn (setf (ewoc--node-right dummy-node) dummy-node)
                       (setf (ewoc--node-left dummy-node) dummy-node)
                       dummy-node))
!          (new-ewoc
!           (ewoc--create (current-buffer)
!                         pretty-printer nil nil dll))
           (pos (point))
           head foot)
      (ewoc--set-buffer-bind-dll new-ewoc
        ;; Set default values
        (unless header (setq header ""))
        (unless footer (setq footer ""))
        (setf (ewoc--node-start-marker dll) (copy-marker pos)
!             foot (ewoc--insert-new-node  dll footer 'insert)
!             head (ewoc--insert-new-node foot header 'insert)
              (ewoc--footer new-ewoc) foot
              (ewoc--header new-ewoc) head))
      ;; Return the ewoc
--- 288,316 ----
  
  Optional second and third arguments HEADER and FOOTER are strings,
  possibly empty, that will always be present at the top and bottom,
! respectively, of the ewoc.
! 
! Normally, a newline is automatically inserted after the header,
! the footer and every node's printed representation.  Optional
! fourth arg NOSEP non-nil inhibits this."
    (let* ((dummy-node (ewoc--node-create 'DL-LIST 'DL-LIST))
           (dll (progn (setf (ewoc--node-right dummy-node) dummy-node)
                       (setf (ewoc--node-left dummy-node) dummy-node)
                       dummy-node))
!          (wrap (if nosep 'identity 'ewoc--wrap))
!          (new-ewoc (ewoc--create (current-buffer)
!                                  (funcall wrap pretty-printer) nil nil dll))
!          (hf-pp (funcall wrap 'insert))
           (pos (point))
           head foot)
+     (setf (ewoc--hf-pp new-ewoc) hf-pp)
      (ewoc--set-buffer-bind-dll new-ewoc
        ;; Set default values
        (unless header (setq header ""))
        (unless footer (setq footer ""))
        (setf (ewoc--node-start-marker dll) (copy-marker pos)
!             foot (ewoc--insert-new-node  dll footer hf-pp)
!             head (ewoc--insert-new-node foot header hf-pp)
              (ewoc--footer new-ewoc) foot
              (ewoc--header new-ewoc) head))
      ;; Return the ewoc
***************
*** 596,607 ****
    "Set the HEADER and FOOTER of EWOC."
    (ewoc--set-buffer-bind-dll-let* ewoc
        ((head (ewoc--header ewoc))
!        (foot (ewoc--footer ewoc)))
      (setf (ewoc--node-data head) header
            (ewoc--node-data foot) footer)
      (save-excursion
!       (ewoc--refresh-node 'insert head)
!       (ewoc--refresh-node 'insert foot))))
  
  \f
  (provide 'ewoc)
--- 615,627 ----
    "Set the HEADER and FOOTER of EWOC."
    (ewoc--set-buffer-bind-dll-let* ewoc
        ((head (ewoc--header ewoc))
!        (foot (ewoc--footer ewoc))
!        (hf-pp (ewoc--hf-pp ewoc)))
      (setf (ewoc--node-data head) header
            (ewoc--node-data foot) footer)
      (save-excursion
!       (ewoc--refresh-node hf-pp head)
!       (ewoc--refresh-node hf-pp foot))))
  
  \f
  (provide 'ewoc)

===end===

Re: ewoc--adjust ugliness

[Stefan Monnier]

> + (defun ewoc--wrap (func)
> +   (lexical-let ((ewoc--user-pp func))
> +     (lambda (data)
> +       (insert-before-markers "\n")
> +       (forward-char -1)
> +       (funcall ewoc--user-pp data)
> +       (forward-char 1))))

Why not

(defun ewoc--wrap (func)
  (lexical-let ((ewoc--user-pp func))
    (lambda (data)
      (funcall ewoc--user-pp data)
      (insert "\n"))))


-- Stefan

Re: ewoc--adjust ugliness

[Thien-Thi Nguyen]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> Why not [more elegant `ewoc--wrap']

indeed, much better!

  * emacs-lisp/ewoc.el (ewoc): Add member `hf-pp' to this structure.
  (ewoc--wrap): New func.
  (ewoc-create): Take additional arg NOSEP.  If nil, wrap node and
  header/footer pretty-printers.  Save header/footer pretty-printer.
  (ewoc-set-hf): Use ewoc's header/footer pretty-printer.

thi

_______________________________________________________________________
diff -c -r1.30 ewoc.el
*** ewoc.el	26 May 2006 08:29:13 -0000	1.30
--- ewoc.el	27 May 2006 08:38:29 -0000
***************
*** 186,192 ****
  	    (:constructor ewoc--create
  			  (buffer pretty-printer header footer dll))
  	    (:conc-name ewoc--))
!   buffer pretty-printer header footer dll last-node)
  
  (defmacro ewoc--set-buffer-bind-dll-let* (ewoc varlist &rest forms)
    "Execute FORMS with ewoc--buffer selected as current buffer,
--- 186,192 ----
  	    (:constructor ewoc--create
  			  (buffer pretty-printer header footer dll))
  	    (:conc-name ewoc--))
!   buffer pretty-printer header footer dll last-node hf-pp)
  
  (defmacro ewoc--set-buffer-bind-dll-let* (ewoc varlist &rest forms)
    "Execute FORMS with ewoc--buffer selected as current buffer,
***************
*** 257,268 ****
      (goto-char m)
      (funcall pp (ewoc--node-data node))
      (ewoc--adjust m (point) R)))
  \f
  ;;; ===========================================================================
  ;;;                  Public members of the Ewoc package
  
  ;;;###autoload
! (defun ewoc-create (pretty-printer &optional header footer)
    "Create an empty ewoc.
  
  The ewoc will be inserted in the current buffer at the current position.
--- 257,275 ----
      (goto-char m)
      (funcall pp (ewoc--node-data node))
      (ewoc--adjust m (point) R)))
+ 
+ (defun ewoc--wrap (func)
+   (lexical-let ((ewoc--user-pp func))
+     (lambda (data)
+       (funcall ewoc--user-pp data)
+       (insert "\n"))))
+ 
  \f
  ;;; ===========================================================================
  ;;;                  Public members of the Ewoc package
  
  ;;;###autoload
! (defun ewoc-create (pretty-printer &optional header footer nosep)
    "Create an empty ewoc.
  
  The ewoc will be inserted in the current buffer at the current position.
***************
*** 275,288 ****
  
  Optional second and third arguments HEADER and FOOTER are strings,
  possibly empty, that will always be present at the top and bottom,
! respectively, of the ewoc."
    (let* ((dummy-node (ewoc--node-create 'DL-LIST 'DL-LIST))
           (dll (progn (setf (ewoc--node-right dummy-node) dummy-node)
                       (setf (ewoc--node-left dummy-node) dummy-node)
                       dummy-node))
!          (new-ewoc
!           (ewoc--create (current-buffer)
!                         pretty-printer nil nil dll))
           (pos (point))
           head foot)
      (ewoc--set-buffer-bind-dll new-ewoc
--- 282,301 ----
  
  Optional second and third arguments HEADER and FOOTER are strings,
  possibly empty, that will always be present at the top and bottom,
! respectively, of the ewoc.
! 
! Normally, a newline is automatically inserted after the header,
! the footer and every node's printed representation.  Optional
! fourth arg NOSEP non-nil inhibits this."
    (let* ((dummy-node (ewoc--node-create 'DL-LIST 'DL-LIST))
           (dll (progn (setf (ewoc--node-right dummy-node) dummy-node)
                       (setf (ewoc--node-left dummy-node) dummy-node)
                       dummy-node))
!          (wrap (if nosep 'identity 'ewoc--wrap))
!          (new-ewoc (ewoc--create (current-buffer)
!                                  (funcall wrap pretty-printer)
!                                  nil nil dll))
!          (hf-pp (funcall wrap 'insert))
           (pos (point))
           head foot)
      (ewoc--set-buffer-bind-dll new-ewoc
***************
*** 290,297 ****
        (unless header (setq header ""))
        (unless footer (setq footer ""))
        (setf (ewoc--node-start-marker dll) (copy-marker pos)
!             foot (ewoc--insert-new-node  dll footer 'insert)
!             head (ewoc--insert-new-node foot header 'insert)
              (ewoc--footer new-ewoc) foot
              (ewoc--header new-ewoc) head))
      ;; Return the ewoc
--- 303,311 ----
        (unless header (setq header ""))
        (unless footer (setq footer ""))
        (setf (ewoc--node-start-marker dll) (copy-marker pos)
!             foot (ewoc--insert-new-node  dll footer hf-pp)
!             head (ewoc--insert-new-node foot header hf-pp)
!             (ewoc--hf-pp new-ewoc) hf-pp
              (ewoc--footer new-ewoc) foot
              (ewoc--header new-ewoc) head))
      ;; Return the ewoc
***************
*** 596,607 ****
    "Set the HEADER and FOOTER of EWOC."
    (ewoc--set-buffer-bind-dll-let* ewoc
        ((head (ewoc--header ewoc))
!        (foot (ewoc--footer ewoc)))
      (setf (ewoc--node-data head) header
            (ewoc--node-data foot) footer)
      (save-excursion
!       (ewoc--refresh-node 'insert head)
!       (ewoc--refresh-node 'insert foot))))
  
  \f
  (provide 'ewoc)
--- 610,622 ----
    "Set the HEADER and FOOTER of EWOC."
    (ewoc--set-buffer-bind-dll-let* ewoc
        ((head (ewoc--header ewoc))
!        (foot (ewoc--footer ewoc))
!        (hf-pp (ewoc--hf-pp ewoc)))
      (setf (ewoc--node-data head) header
            (ewoc--node-data foot) footer)
      (save-excursion
!       (ewoc--refresh-node hf-pp head)
!       (ewoc--refresh-node hf-pp foot))))
  
  \f
  (provide 'ewoc)

Re: ewoc--adjust ugliness

[Sam Steingold]

> * Mathias Dahl <znguvnf.qnuy@tznvy.pbz> [2006-05-26 12:49:59 +0200]:
>
>> > ewoc--adjust refers to a free variable `dll'.  This is not
>> > clean, because that name is short and has no defvar.
>>
>> thanks for pointing this out.  i have installed a fix.
>
> There seems to be quite a few things happening with ewoc right now,
> has anyone looked into the recent bug in ewoc that Magnus found? See
> below:
>
> --
>
> From: Magnus Henoch <mange@freemail.hu>
> Subject: ewoc misbehaves
> Newsgroups: gmane.emacs.pretest.bugs
> Date: Sun, 21 May 2006 16:14:00 +0200
>
> Revision 1.26 of ewoc.el changes its behavior in
> backwards-incompatible ways.  Specifically:
>
> - A newline is no longer inserted between items, and after header and
>  footer.
>
> - Markers after the ewoc structure are not advanced, but suddenly
>  point before it after an item is inserted.
>
> Magnus
>
> --
>
> No one has responded, as far as I can see, but maybe someone is
> working on it? I for one cannot use the latest CVS Emacs because of
> this as it breaks the nice jabber.el package.
>
> Sorry if this seems like nagging (it is, kind of... :).

also, I need to manually re-load ewoc to use pcvs.
http://article.gmane.org/gmane.emacs.devel:54832
http://article.gmane.org/gmane.emacs.devel:55034

-- 
Sam Steingold (http://www.podval.org/~sds) on Fedora Core release 5 (Bordeaux)
http://mideasttruth.com http://memri.org http://truepeace.org http://pmw.org.il
http://iris.org.il http://camera.org http://jihadwatch.org http://dhimmi.com
186,000 Miles per Second.  It's not just a good idea.  IT'S THE LAW.