unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* PATCH: ewoc.el to permit node representation abutment
@ 2006-05-05 12:03 Thien-Thi Nguyen
  2006-05-05 13:20 ` Stefan Monnier
  0 siblings, 1 reply; 6+ messages in thread
From: Thien-Thi Nguyen @ 2006-05-05 12:03 UTC (permalink / raw)


ewoc.el is pretty cool but as it stands it inserts a gratuitous newline
between each node's "pretty-printed" representation.  this makes it
unsuitable for single-line usage, a design bug in my book.  i've locally
modified ewoc.el to allow abutment of a node's representation, and
correspondingly its only in-emacs client, as shown by the appended
patch.  here is a suitable ChangeLog entry:

	* emacs-lisp/ewoc.el (ewoc--create-node): Don't insert
	trailing newline.  Also, create marker with insertion type t.
	(ewoc--refresh-node): Delete all text from current node's
	start marker to the next one's.  Also, temporarily set the
	marker's insertion type to nil around the pretty-printer call.
	(ewoc-create): Use marker insertion type t.  Elide two lambdas.
	(ewoc-refresh): Don't insert newline.  Also, temporarily set the
	marker's insertion type to nil around the pretty-printer call.
	* pcvs-info.el (cvs-fileinfo-pp): Insert trailing newline.

please review (especially pcvs behavior -- i tested lightly with, but
don't normally use, pcvs).  if there are no objections i will commit it
in a week or so (w/ NEWS blurb).  if anyone has written or has started
writing docs for ewoc.el (presumably for lispref/ inclusion), please
speak up else i will be doing that in the meantime.

thi


___________________________________________________________________________
-*- mode: compilation; default-directory: "~/build/GNU/emacs/lisp/" -*-
Compilation started at Fri May  5 13:47:14

for f in pcvs-info.el emacs-lisp/ewoc.el ; do diff -c $f $f.NEW ; done
*** pcvs-info.el	Thu May  4 15:29:51 2006
--- pcvs-info.el.NEW	Fri May  5 12:43:07 2006
***************
*** 379,385 ****
  		      ;; or nothing
  		      "")))
  	   (format "%-11s %s %-11s %-11s %s"
! 		   side status type base file)))))))
  
  
  (defun cvs-fileinfo-update (fi fi-new)
--- 379,386 ----
  		      ;; or nothing
  		      "")))
  	   (format "%-11s %s %-11s %-11s %s"
! 		   side status type base file))))
!      "\n")))
  
  
  (defun cvs-fileinfo-update (fi fi-new)
*** emacs-lisp/ewoc.el	Mon Feb  6 13:14:52 2006
--- emacs-lisp/ewoc.el.NEW	Fri May  5 12:34:52 2006
***************
*** 253,265 ****
      (when (markerp pos) (setq pos (marker-position pos)))
      (goto-char pos)
      (let ((inhibit-read-only t))
-       ;; Insert the trailing newline using insert-before-markers
-       ;; so that the start position for the next element is updated.
-       (insert-before-markers ?\n)
-       ;; Move back, and call the pretty-printer.
-       (backward-char 1)
        (funcall pretty-printer data)
!       (ewoc--node-create (copy-marker pos) data))))
  
  
  (defun ewoc--delete-node-internal (ewoc node)
--- 253,260 ----
      (when (markerp pos) (setq pos (marker-position pos)))
      (goto-char pos)
      (let ((inhibit-read-only t))
        (funcall pretty-printer data)
!       (ewoc--node-create (copy-marker pos t) data))))
  
  
  (defun ewoc--delete-node-internal (ewoc node)
***************
*** 287,297 ****
      (save-excursion
        ;; First, remove the string from the buffer:
        (delete-region (ewoc--node-start-marker node)
! 		     (1- (marker-position
! 			  (ewoc--node-start-marker (ewoc--node-right node)))))
        ;; Calculate and insert the string.
!       (goto-char (ewoc--node-start-marker node))
!       (funcall pp (ewoc--node-data node)))))
  \f
  ;;; ===========================================================================
  ;;;                  Public members of the Ewoc package
--- 282,294 ----
      (save-excursion
        ;; First, remove the string from the buffer:
        (delete-region (ewoc--node-start-marker node)
!                      (ewoc--node-start-marker (ewoc--node-right node)))
        ;; Calculate and insert the string.
!       (let ((m (ewoc--node-start-marker node)))
!         (goto-char m)
!         (set-marker-insertion-type m nil)
!         (funcall pp (ewoc--node-data node))
!         (set-marker-insertion-type m t)))))
  \f
  ;;; ===========================================================================
  ;;;                  Public members of the Ewoc package
***************
*** 321,329 ****
        ;; Set default values
        (unless header (setq header ""))
        (unless footer (setq footer ""))
!       (setf (ewoc--node-start-marker dll) (copy-marker pos))
!       (let ((foot (ewoc--create-node footer (lambda (x) (insert footer)) pos))
! 	    (head (ewoc--create-node header (lambda (x) (insert header)) pos)))
  	(ewoc--node-enter-first dll head)
  	(ewoc--node-enter-last  dll foot)
  	(setf (ewoc--header new-ewoc) head)
--- 318,326 ----
        ;; Set default values
        (unless header (setq header ""))
        (unless footer (setq footer ""))
!       (setf (ewoc--node-start-marker dll) (copy-marker pos t))
!       (let ((foot (ewoc--create-node footer 'insert pos))
! 	    (head (ewoc--create-node header 'insert pos)))
  	(ewoc--node-enter-first dll head)
  	(ewoc--node-enter-last  dll foot)
  	(setf (ewoc--header new-ewoc) head)
***************
*** 555,567 ****
        (delete-region (ewoc--node-start-marker (ewoc--node-nth dll 1))
  		     (ewoc--node-start-marker footer))
        (goto-char (ewoc--node-start-marker footer))
!       (let ((node (ewoc--node-nth dll 1)))
  	(while (not (eq node footer))
! 	  (set-marker (ewoc--node-start-marker node) (point))
! 	  (funcall (ewoc--pretty-printer ewoc)
! 		   (ewoc--node-data node))
! 	  (insert "\n")
! 	  (setq node (ewoc--node-next dll node)))))
      (set-marker (ewoc--node-start-marker footer) (point))))
  
  (defun ewoc-collect (ewoc predicate &rest args)
--- 552,567 ----
        (delete-region (ewoc--node-start-marker (ewoc--node-nth dll 1))
  		     (ewoc--node-start-marker footer))
        (goto-char (ewoc--node-start-marker footer))
!       (let* ((pp (ewoc--pretty-printer ewoc))
!              (node (ewoc--node-nth dll 1))
!              (m (ewoc--node-start-marker node)))
  	(while (not (eq node footer))
! 	  (set-marker m (point))
!           (set-marker-insertion-type m nil)
! 	  (funcall pp (ewoc--node-data node))
!           (set-marker-insertion-type m t)
! 	  (setq node (ewoc--node-next dll node)
!                 m (ewoc--node-start-marker node)))))
      (set-marker (ewoc--node-start-marker footer) (point))))
  
  (defun ewoc-collect (ewoc predicate &rest args)

Compilation exited abnormally with code 1 at Fri May  5 13:47:14

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: PATCH: ewoc.el to permit node representation abutment
  2006-05-05 12:03 PATCH: ewoc.el to permit node representation abutment Thien-Thi Nguyen
@ 2006-05-05 13:20 ` Stefan Monnier
  2006-05-05 21:43   ` ttn
  0 siblings, 1 reply; 6+ messages in thread
From: Stefan Monnier @ 2006-05-05 13:20 UTC (permalink / raw)
  Cc: emacs-devel

> ewoc.el is pretty cool but as it stands it inserts a gratuitous newline
> between each node's "pretty-printed" representation.  this makes it
> unsuitable for single-line usage, a design bug in my book.  i've locally
> modified ewoc.el to allow abutment of a node's representation, and
> correspondingly its only in-emacs client, as shown by the appended
> patch.  here is a suitable ChangeLog entry:

Cool.  I assume you tested it with the most important case: when the
pretty-printer returns the empty string (so you can temporarily hide
elements, which is the main reason why I wanted this feature implemented).

> 	* emacs-lisp/ewoc.el (ewoc--create-node): Don't insert
> 	trailing newline.  Also, create marker with insertion type t.
> 	(ewoc--refresh-node): Delete all text from current node's
> 	start marker to the next one's.  Also, temporarily set the
> 	marker's insertion type to nil around the pretty-printer call.

Ah, setting the marker's insertion type is a great idea.

The problem case is when we have at the same spot many empty elements, and
the one we refresh is one of the middle ones, so the preceding ones
shouldn't move but the subsequent ones should.  It seems your code doesn't
pay attention to this detail and will either move the preceding ones as well
or fail to move the subsequent ones.

> 	(ewoc-create): Use marker insertion type t.  Elide two lambdas.

Your "elide a lambda" is pretty much an "eta reduction".  Thanks for
spotting it.

> !       (let* ((pp (ewoc--pretty-printer ewoc))
> !              (node (ewoc--node-nth dll 1))
> !              (m (ewoc--node-start-marker node)))
>   	(while (not (eq node footer))
> ! 	  (set-marker m (point))
> !           (set-marker-insertion-type m nil)
> ! 	  (funcall pp (ewoc--node-data node))
> !           (set-marker-insertion-type m t)
> ! 	  (setq node (ewoc--node-next dll node)
> !                 m (ewoc--node-start-marker node)))))
>       (set-marker (ewoc--node-start-marker footer) (point))))
  
Please move the let-declaration of `m' assignment to right after the `while'
rather than duplicating it (one before the while, the other at its end).


        Stefan

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: PATCH: ewoc.el to permit node representation abutment
  2006-05-05 13:20 ` Stefan Monnier
@ 2006-05-05 21:43   ` ttn
  2006-05-08  8:48     ` Thien-Thi Nguyen
  0 siblings, 1 reply; 6+ messages in thread
From: ttn @ 2006-05-05 21:43 UTC (permalink / raw)
  Cc: emacs-devel

   From: Stefan Monnier <monnier@iro.umontreal.ca>
   Date: Fri, 05 May 2006 09:20:08 -0400

   Cool.  I assume you tested it with the most important case: when the
   pretty-printer returns the empty string (so you can temporarily hide
   elements, which is the main reason why I wanted this feature
   implemented).

actually, i didn't test that case (having in mind the munging of text
properties and `buffer-invisibility-spec' to implement hiding, instead).
i just started pcvs on a smallish project and typed `g' a few times.
anyway, looks like it will be handled in the process of solving...

   The problem case is when we have at the same spot many empty
   elements, and the one we refresh is one of the middle ones, so the
   preceding ones shouldn't move but the subsequent ones should.  It
   seems your code doesn't pay attention to this detail and will either
   move the preceding ones as well or fail to move the subsequent ones.

thanks for illustrating a concrete failure scenario.  i had vague doubts
but now they are firmer.  looks like slightly more hair is called for.

   [eta reduction]

it would merit "bugfix" if the header and footer were ever refreshed,
but i see that `ewoc-refresh' avoids them -- ugh.  (there seem to be
many design enhancement opportunities in ewoc.el...)

thi

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: PATCH: ewoc.el to permit node representation abutment
  2006-05-05 21:43   ` ttn
@ 2006-05-08  8:48     ` Thien-Thi Nguyen
  2006-05-08 15:04       ` Stefan Monnier
  0 siblings, 1 reply; 6+ messages in thread
From: Thien-Thi Nguyen @ 2006-05-08  8:48 UTC (permalink / raw)


i've revised the patch (please see below) to handle empty representations.

this was tested by modifying `cvs-fileinfo-pp' to output empty string for a
certain hardcoded filename in my local repo, making the empty string
conditional on a variable and toggling the variable followed by either
`ewoc-refresh' or `ewoc-invalidate' calls.  (this description omits several
steps for preparing the testing framework, but you get the idea.)

	* emacs-lisp/ewoc.el (ewoc--create-node): Don't insert
	trailing newline.  Also, create marker with insertion type t.
	(ewoc--refresh-node): Delete all text from current node's
	start marker to the next one's.  Also, make the marker
	"stay put" before calling the pretty-printer, and return it.
	(ewoc--init-stay-put-list, ewoc--reset-marker-types): New funcs.
	(ewoc-create): Use marker insertion type t.
	(ewoc-map): Collect "stay put" markers; reset them when done.
	(ewoc-invalidate): Rewrite.
	(ewoc-refresh): Don't insert newline.
	Collect "stay put" markers; reset them when done.
	(ewoc-set-hf): Reset header and footer markers when done.
	* pcvs-info.el (cvs-fileinfo-pp): Insert trailing newline.

note that ewoc.el is 1.19 from cvs.

thi

_________________________________________________________
diff -c -F '^(' ewoc.el ewoc.el.NEW
*** ewoc.el	Mon May  8 10:00:48 2006
--- ewoc.el.NEW	Mon May  8 10:11:45 2006
*************** (defun ewoc--create-node (data pretty-pr
*** 249,261 ****
      (when (markerp pos) (setq pos (marker-position pos)))
      (goto-char pos)
      (let ((inhibit-read-only t))
-       ;; Insert the trailing newline using insert-before-markers
-       ;; so that the start position for the next element is updated.
-       (insert-before-markers ?\n)
-       ;; Move back, and call the pretty-printer.
-       (backward-char 1)
        (funcall pretty-printer data)
!       (ewoc--node-create (copy-marker pos) data))))
  
  
  (defun ewoc--delete-node-internal (ewoc node)
--- 249,256 ----
      (when (markerp pos) (setq pos (marker-position pos)))
      (goto-char pos)
      (let ((inhibit-read-only t))
        (funcall pretty-printer data)
!       (ewoc--node-create (copy-marker pos t) data))))
  
  
  (defun ewoc--delete-node-internal (ewoc node)
*************** (defun ewoc--delete-node-internal (ewoc 
*** 276,293 ****
      ;; Delete the node, and return the wrapper.
      (ewoc--node-delete node)))
  
  
  (defun ewoc--refresh-node (pp node)
!   "Redisplay the element represented by NODE using the pretty-printer PP."
    (let ((inhibit-read-only t))
      (save-excursion
        ;; First, remove the string from the buffer:
        (delete-region (ewoc--node-start-marker node)
! 		     (1- (marker-position
! 			  (ewoc--node-start-marker (ewoc--node-right node)))))
        ;; Calculate and insert the string.
!       (goto-char (ewoc--node-start-marker node))
!       (funcall pp (ewoc--node-data node)))))
  \f
  ;;; ===========================================================================
  ;;;                  Public members of the Ewoc package
--- 271,317 ----
      ;; Delete the node, and return the wrapper.
      (ewoc--node-delete node)))
  
+ ;; Refresh protocol
+ ;;
+ ;; Refresh requires essentially two passes.  The first collects info on which
+ ;; nodes have been visited.  The header is always visited, the footer never.
+ ;; For each node visited in order of increasing position: (1a) set marker
+ ;; insertion type to nil; (1b) do the pretty-printing (if necessary).  Then,
+ ;; when all pretty-printing is done: (2) reset all insertion types to t.
+ ;;
+ ;; This approach ensures that a series of prior nodes (including the header)
+ ;; can insert nothing (empty string) and maintain their start markers (aka
+ ;; "stay put") should a later node insert something (non-empty string) at the
+ ;; same position.  Note: "series" for prior and "a" for later because when
+ ;; something is inserted, that changes the position.  This property explains
+ ;; the "if necessary" in the previous paragraph; for partial refresh (i.e.,
+ ;; `ewoc-invalidate') we must visit those nodes prior to the ones requested
+ ;; iff they have the same position even though we do not pretty-print them.
  
  (defun ewoc--refresh-node (pp node)
!   "Redisplay the element represented by NODE using the pretty-printer PP.
! Set NODE's start marker's insertion type to nil and return it."
    (let ((inhibit-read-only t))
      (save-excursion
        ;; First, remove the string from the buffer:
        (delete-region (ewoc--node-start-marker node)
!                      (ewoc--node-start-marker (ewoc--node-right node)))
        ;; Calculate and insert the string.
!       (let ((m (ewoc--node-start-marker node)))
!         (goto-char m)
!         (set-marker-insertion-type m nil)
!         (funcall pp (ewoc--node-data node))
!         m))))
! 
! (defun ewoc--init-stay-put-list (ewoc)
!   (let ((m (ewoc--node-start-marker (ewoc--header ewoc))))
!     (set-marker-insertion-type m nil)
!     (list m)))
! 
! (defsubst ewoc--reset-marker-types (list)
!   (dolist (m list)
!     (set-marker-insertion-type m t)))
! 
  \f
  ;;; ===========================================================================
  ;;;                  Public members of the Ewoc package
*************** (defun ewoc-create (pretty-printer &opti
*** 301,308 ****
  PRETTY-PRINTER should be a function that takes one argument, an
  element, and inserts a string representing it in the buffer (at
  point).  The string PRETTY-PRINTER inserts may be empty or span
! several lines.  A trailing newline will always be inserted
! automatically.  The PRETTY-PRINTER should use `insert', and not
  `insert-before-markers'.
  
  Optional second argument HEADER is a string that will always be
--- 325,331 ----
  PRETTY-PRINTER should be a function that takes one argument, an
  element, and inserts a string representing it in the buffer (at
  point).  The string PRETTY-PRINTER inserts may be empty or span
! several lines.  The PRETTY-PRINTER should use `insert', and not
  `insert-before-markers'.
  
  Optional second argument HEADER is a string that will always be
*************** (defun ewoc-create (pretty-printer &opti
*** 317,323 ****
        ;; Set default values
        (unless header (setq header ""))
        (unless footer (setq footer ""))
!       (setf (ewoc--node-start-marker dll) (copy-marker pos))
        (let ((foot (ewoc--create-node footer 'insert pos))
  	    (head (ewoc--create-node header 'insert pos)))
  	(ewoc--node-enter-first dll head)
--- 340,346 ----
        ;; Set default values
        (unless header (setq header ""))
        (unless footer (setq footer ""))
!       (setf (ewoc--node-start-marker dll) (copy-marker pos t))
        (let ((foot (ewoc--create-node footer 'insert pos))
  	    (head (ewoc--create-node header 'insert pos)))
  	(ewoc--node-enter-first dll head)
*************** (defun ewoc-map (map-function ewoc &rest
*** 400,411 ****
  If more than two arguments are given, the remaining
  arguments will be passed to MAP-FUNCTION."
    (ewoc--set-buffer-bind-dll-let* ewoc
!       ((footer (ewoc--footer ewoc))
!        (node (ewoc--node-nth dll 1)))
      (while (not (eq node footer))
        (if (apply map-function (ewoc--node-data node) args)
! 	  (ewoc--refresh-node (ewoc--pretty-printer ewoc) node))
!       (setq node (ewoc--node-next dll node)))))
  
  (defun ewoc-filter (ewoc predicate &rest args)
    "Remove all elements in EWOC for which PREDICATE returns nil.
--- 423,437 ----
  If more than two arguments are given, the remaining
  arguments will be passed to MAP-FUNCTION."
    (ewoc--set-buffer-bind-dll-let* ewoc
!       ((pp (ewoc--pretty-printer ewoc))
!        (footer (ewoc--footer ewoc))
!        (node (ewoc--node-nth dll 1))
!        (stay-put (ewoc--init-stay-put-list ewoc)))
      (while (not (eq node footer))
        (if (apply map-function (ewoc--node-data node) args)
!           (push (ewoc--refresh-node pp node) stay-put))
!       (setq node (ewoc--node-next dll node)))
!     (ewoc--reset-marker-types stay-put)))
  
  (defun ewoc-filter (ewoc predicate &rest args)
    "Remove all elements in EWOC for which PREDICATE returns nil.
*************** (defun ewoc-locate (ewoc &optional pos g
*** 497,505 ****
  (defun ewoc-invalidate (ewoc &rest nodes)
    "Call EWOC's pretty-printer for each element in NODES.
  Delete current text first, thus effecting a \"refresh\"."
!   (ewoc--set-buffer-bind-dll ewoc
!     (dolist (node nodes)
!       (ewoc--refresh-node (ewoc--pretty-printer ewoc) node))))
  
  (defun ewoc-goto-prev (ewoc arg)
    "Move point to the ARGth previous element in EWOC.
--- 523,562 ----
  (defun ewoc-invalidate (ewoc &rest nodes)
    "Call EWOC's pretty-printer for each element in NODES.
  Delete current text first, thus effecting a \"refresh\"."
!   (ewoc--set-buffer-bind-dll-let* ewoc
!       ((pp (ewoc--pretty-printer ewoc))
!        (header (ewoc--header ewoc))
!        (stay-put (ewoc--init-stay-put-list ewoc)))
!     (setq nodes
!           (sort nodes
!                 (lambda (a b)
!                   (let ((am (ewoc--node-start-marker a))
!                         (bm (ewoc--node-start-marker b)))
!                     (cond ((> am bm) nil)
!                           ((< am bm) t)
!                           (t ;; i.e., (= am bm)
!                            ;; Maintain DLL ordering: A is "less than" B
!                            ;; if is found closer to the header than B.
!                            (let (donep lessp)
!                              (while (not (or donep
!                                              (eq header
!                                                  (setq b (ewoc--node-left
!                                                           b)))))
!                                (when (eq a b)
!                                  (setq lessp t donep t)))
!                              lessp)))))))
!     (let ((stop (ewoc--header ewoc))
!           node-m mid mid-m)
!       (dolist (node nodes)
!         (setq node-m (ewoc--node-start-marker node)
!               mid node)
!         (while (and (not (eq stop (setq mid (ewoc--node-left mid))))
!                     (= node-m (setq mid-m (ewoc--node-start-marker mid))))
!           (set-marker-insertion-type mid-m nil)
!           (push mid-m stay-put))
!         (push (ewoc--refresh-node pp node) stay-put)
!         (setq stop node)))
!     (ewoc--reset-marker-types stay-put)))
  
  (defun ewoc-goto-prev (ewoc arg)
    "Move point to the ARGth previous element in EWOC.
*************** (defun ewoc-refresh (ewoc)
*** 551,563 ****
        (delete-region (ewoc--node-start-marker (ewoc--node-nth dll 1))
  		     (ewoc--node-start-marker footer))
        (goto-char (ewoc--node-start-marker footer))
!       (let ((node (ewoc--node-nth dll 1)))
  	(while (not (eq node footer))
! 	  (set-marker (ewoc--node-start-marker node) (point))
! 	  (funcall (ewoc--pretty-printer ewoc)
! 		   (ewoc--node-data node))
! 	  (insert "\n")
! 	  (setq node (ewoc--node-next dll node)))))
      (set-marker (ewoc--node-start-marker footer) (point))))
  
  (defun ewoc-collect (ewoc predicate &rest args)
--- 608,625 ----
        (delete-region (ewoc--node-start-marker (ewoc--node-nth dll 1))
  		     (ewoc--node-start-marker footer))
        (goto-char (ewoc--node-start-marker footer))
!       (let* ((pp (ewoc--pretty-printer ewoc))
!              (node (ewoc--node-nth dll 1))
!              (stay-put (ewoc--init-stay-put-list ewoc))
!              m)
  	(while (not (eq node footer))
!           (setq m (ewoc--node-start-marker node))
! 	  (set-marker m (point))
!           (set-marker-insertion-type m nil)
!           (push m stay-put)
! 	  (funcall pp (ewoc--node-data node))
! 	  (setq node (ewoc--node-next dll node)))
!         (ewoc--reset-marker-types stay-put)))
      (set-marker (ewoc--node-start-marker footer) (point))))
  
  (defun ewoc-collect (ewoc predicate &rest args)
*************** (defun ewoc-set-hf (ewoc header footer)
*** 597,604 ****
    "Set the HEADER and FOOTER of EWOC."
    (setf (ewoc--node-data (ewoc--header ewoc)) header)
    (setf (ewoc--node-data (ewoc--footer ewoc)) footer)
!   (ewoc--refresh-node 'insert (ewoc--header ewoc))
!   (ewoc--refresh-node 'insert (ewoc--footer ewoc)))
  
  \f
  (provide 'ewoc)
--- 659,668 ----
    "Set the HEADER and FOOTER of EWOC."
    (setf (ewoc--node-data (ewoc--header ewoc)) header)
    (setf (ewoc--node-data (ewoc--footer ewoc)) footer)
!   (let ((stay-put (list
!                    (ewoc--refresh-node 'insert (ewoc--header ewoc))
!                    (ewoc--refresh-node 'insert (ewoc--footer ewoc)))))
!     (ewoc--reset-marker-types stay-put)))
  
  \f
  (provide 'ewoc)

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: PATCH: ewoc.el to permit node representation abutment
  2006-05-08  8:48     ` Thien-Thi Nguyen
@ 2006-05-08 15:04       ` Stefan Monnier
  2006-05-13  5:08         ` Thien-Thi Nguyen
  0 siblings, 1 reply; 6+ messages in thread
From: Stefan Monnier @ 2006-05-08 15:04 UTC (permalink / raw)
  Cc: emacs-devel

> i've revised the patch (please see below) to handle empty representations.
> this was tested by modifying `cvs-fileinfo-pp' to output empty string for a
> certain hardcoded filename in my local repo, making the empty string
> conditional on a variable and toggling the variable followed by either
> `ewoc-refresh' or `ewoc-invalidate' calls.  (this description omits several
> steps for preparing the testing framework, but you get the idea.)

> 	* emacs-lisp/ewoc.el (ewoc--create-node): Don't insert
> 	trailing newline.  Also, create marker with insertion type t.
> 	(ewoc--refresh-node): Delete all text from current node's
> 	start marker to the next one's.  Also, make the marker
> 	"stay put" before calling the pretty-printer, and return it.
> 	(ewoc--init-stay-put-list, ewoc--reset-marker-types): New funcs.
> 	(ewoc-create): Use marker insertion type t.
> 	(ewoc-map): Collect "stay put" markers; reset them when done.
> 	(ewoc-invalidate): Rewrite.
> 	(ewoc-refresh): Don't insert newline.
> 	Collect "stay put" markers; reset them when done.
> 	(ewoc-set-hf): Reset header and footer markers when done.
> 	* pcvs-info.el (cvs-fileinfo-pp): Insert trailing newline.

> note that ewoc.el is 1.19 from cvs.

Hmm... now it looks too complex to be installed before Emacs-22.
Can't it be done more simply.  Basically just do something like:
[ quality-control: this is guaranteed 100% untested ]

 (defun ewoc--refresh-node (pp node)
   "Redisplay the element represented by NODE using the pretty-printer PP.
 Set NODE's start marker's insertion type to nil and return it."
   (save-excursion
     (let* ((inhibit-read-only t)
            (m (ewoc--node-start-marker node))
            (n (ewoc--node-right node)))
       ;; First, remove the string from the buffer:
       (delete-region m (ewoc--node-start-marker n))
       ;; Calculate and insert the string.
       (goto-char m)
       ;; Temporarily change insertion type of the end marker so it is indeed
       ;; moved to the end.
       (set-marker-insertion-type (ewoc--node-start-marker n) t)
       (funcall pp (ewoc--node-data node))
       (set-marker-insertion-type (ewoc--node-start-marker n) nil)
       ;; Since our markers have insertion type nil (i.e. not "insert-before"),
       ;; all markers that were at position `m' are still at position `m',
       ;; including the ones for subsequent empty nodes, which we thus have
       ;; to reseat.
       (setq m (ewoc--node-start-marker n))
       (while (and (setq n (ewoc--node-right n))
                   (< (ewoc--node-start-marker n) m))
           (set-marker (ewoc--node-start-marker n) m))))))


-- Stefan

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: PATCH: ewoc.el to permit node representation abutment
  2006-05-08 15:04       ` Stefan Monnier
@ 2006-05-13  5:08         ` Thien-Thi Nguyen
  0 siblings, 0 replies; 6+ messages in thread
From: Thien-Thi Nguyen @ 2006-05-13  5:08 UTC (permalink / raw)


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

> Hmm... now it looks too complex to be installed before Emacs-22.

yeah, looks like toggling marker insertion type did not prove to be so
clean in practice.  i've installed a few (hopefully uncontroversial)
implementation touch-ups to ewoc.el (now at cvs revision 1.23) and have
prepared the following patch against it for review.

this one differs from the previous two in that pcvs.el is also involved.
(it's omission before shows a testing methodology bug, argh...)  as
before, changes to pcvs*.el are to preserve "look and feel" from the
user's perspective.

here is a suitable ChangeLog entry:

	* emacs-lisp/ewoc.el (ewoc--adjust): New func.
	(ewoc--insert-new-node): Don't insert trailing newline.
	Instead, adjust succesor nodes' start markers.
	(ewoc--refresh-node): Delete all text from current node's start
	marker to the next one's; adjust successor nodes' start markers.
	(ewoc--create): Doc fix.
	(ewoc--refresh): Don't insert newline.
	(ewoc--set-hf): Use `ewoc--set-buffer-bind-dll-let*'.
	* pcvs.el (cvs-make-cvs-buffer): Specify
	extra newline for ewoc's header and footer.
	(cvs-update-header): Update initial header recognition.
	Append newline to final header and footer values.
	* pcvs-info.el (cvs-fileinfo-pp): Insert trailing newline.

i will be off net for a few days but will be back to address comments
and hopefully wrap things up early next week.

thi


__________________________________________________________________
*** emacs-lisp/ewoc.el	Fri May 12 13:36:31 2006
--- emacs-lisp/ewoc.el.NEW	Sat May 13 06:11:24 2006
*************** (defsubst ewoc--filter-hf-nodes (ewoc no
*** 205,210 ****
--- 205,226 ----
  	      (eq node (ewoc--footer ewoc)))
      node))
  
+ (defun ewoc--adjust (beg end node)
+   ;; "Manually reseat" markers for NODE and its successors (including footer
+   ;; and dll), in the case where they originally shared start position with
+   ;; BEG, to END.  BEG and END are buffer positions describing NODE's left
+   ;; neighbor.  This operation is functionally equivalent to temporarily
+   ;; setting these nodes' markers' insertion type to t around the pretty-print
+   ;; call that precedes the call to `ewoc-adjust', and then changing them back
+   ;; to nil.
+   (when (< beg end)
+     (let (m)
+       (while (and (= beg (setq m (ewoc--node-start-marker node)))
+                   (progn
+                     (set-marker m end)
+                     (not (eq dll node))))
+         (setq node (ewoc--node-right node))))))
+ 
  (defun ewoc--insert-new-node (node data pretty-printer)
    "Insert before NODE a new node for DATA, displayed by PRETTY-PRINTER.
  Call PRETTY-PRINTER with point at NODE's start, thus pushing back
*************** (defun ewoc--insert-new-node (node data 
*** 215,243 ****
             (pos (marker-position m))
             (elemnode (ewoc--node-create m data)))
        (goto-char pos)
-       ;; Insert the trailing newline using insert-before-markers
-       ;; so that the start position for the next element is updated.
-       (insert-before-markers ?\n)
-       ;; Move back, and call the pretty-printer.
-       (backward-char 1)
        (funcall pretty-printer data)
        (setf (marker-position m) pos
              (ewoc--node-left  elemnode) (ewoc--node-left node)
              (ewoc--node-right elemnode)                  node
              (ewoc--node-right (ewoc--node-left node)) elemnode
              (ewoc--node-left                   node)  elemnode)
        elemnode)))
  
  (defun ewoc--refresh-node (pp node)
    "Redisplay the element represented by NODE using the pretty-printer PP."
!   (let ((inhibit-read-only t))
      ;; First, remove the string from the buffer:
!     (delete-region (ewoc--node-start-marker node)
!                    (1- (marker-position
!                         (ewoc--node-start-marker (ewoc--node-right node)))))
      ;; Calculate and insert the string.
!     (goto-char (ewoc--node-start-marker node))
!     (funcall pp (ewoc--node-data node))))
  \f
  ;;; ===========================================================================
  ;;;                  Public members of the Ewoc package
--- 231,256 ----
             (pos (marker-position m))
             (elemnode (ewoc--node-create m data)))
        (goto-char pos)
        (funcall pretty-printer data)
        (setf (marker-position m) pos
              (ewoc--node-left  elemnode) (ewoc--node-left node)
              (ewoc--node-right elemnode)                  node
              (ewoc--node-right (ewoc--node-left node)) elemnode
              (ewoc--node-left                   node)  elemnode)
+       (ewoc--adjust pos (point) node)
        elemnode)))
  
  (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
*************** (defun ewoc-create (pretty-printer &opti
*** 251,258 ****
  PRETTY-PRINTER should be a function that takes one argument, an
  element, and inserts a string representing it in the buffer (at
  point).  The string PRETTY-PRINTER inserts may be empty or span
! several lines.  A trailing newline will always be inserted
! automatically.  The PRETTY-PRINTER should use `insert', and not
  `insert-before-markers'.
  
  Optional second argument HEADER is a string that will always be
--- 264,270 ----
  PRETTY-PRINTER should be a function that takes one argument, an
  element, and inserts a string representing it in the buffer (at
  point).  The string PRETTY-PRINTER inserts may be empty or span
! several lines.  The PRETTY-PRINTER should use `insert', and not
  `insert-before-markers'.
  
  Optional second argument HEADER is a string that will always be
*************** (defun ewoc-refresh (ewoc)
*** 522,528 ****
  	(while (not (eq node footer))
  	  (set-marker (ewoc--node-start-marker node) (point))
  	  (funcall pp (ewoc--node-data node))
- 	  (insert "\n")
  	  (setq node (ewoc--node-next dll node)))))
      (set-marker (ewoc--node-start-marker footer) (point))))
  
--- 534,539 ----
*************** (defun ewoc-get-hf (ewoc)
*** 561,571 ****
  
  (defun ewoc-set-hf (ewoc header footer)
    "Set the HEADER and FOOTER of EWOC."
!   (setf (ewoc--node-data (ewoc--header ewoc)) header)
!   (setf (ewoc--node-data (ewoc--footer ewoc)) footer)
!   (save-excursion
!     (ewoc--refresh-node 'insert (ewoc--header ewoc))
!     (ewoc--refresh-node 'insert (ewoc--footer ewoc))))
  
  \f
  (provide 'ewoc)
--- 572,585 ----
  
  (defun ewoc-set-hf (ewoc header footer)
    "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)
*** pcvs.el	Sat May 13 06:16:33 2006
--- pcvs.el.NEW	Sat May 13 06:35:42 2006
*************** (defun cvs-make-cvs-buffer (dir &optiona
*** 467,473 ****
  	 (cvs-mode)
  	 (set (make-local-variable 'list-buffers-directory) buffer-name)
  	 ;;(set (make-local-variable 'cvs-temp-buffer) (cvs-temp-buffer))
! 	 (let ((cookies (ewoc-create 'cvs-fileinfo-pp "\n" "")))
  	   (set (make-local-variable 'cvs-cookies) cookies)
  	   (add-hook 'kill-buffer-hook
  		     (lambda ()
--- 467,473 ----
  	 (cvs-mode)
  	 (set (make-local-variable 'list-buffers-directory) buffer-name)
  	 ;;(set (make-local-variable 'cvs-temp-buffer) (cvs-temp-buffer))
! 	 (let ((cookies (ewoc-create 'cvs-fileinfo-pp "\n\n" "\n")))
  	   (set (make-local-variable 'cvs-cookies) cookies)
  	   (add-hook 'kill-buffer-hook
  		     (lambda ()
*************** (defun cvs-update-header (cmd add)
*** 618,624 ****
  	 (str (car hf))
  	 (done "")
  	 (tin (ewoc-nth cvs-cookies 0)))
!     (if (eq (length str) 1) (setq str ""))
      ;; look for the first *real* fileinfo (to determine emptyness)
      (while
  	(and tin
--- 618,624 ----
  	 (str (car hf))
  	 (done "")
  	 (tin (ewoc-nth cvs-cookies 0)))
!     (if (eq (length str) 2) (setq str ""))
      ;; look for the first *real* fileinfo (to determine emptyness)
      (while
  	(and tin
*************** (defun cvs-update-header (cmd add)
*** 633,638 ****
--- 633,639 ----
  	(setq str (replace-match "" t t str))
  	(if (zerop (length str)) (setq str "\n"))
  	(setq done (concat "-- last cmd: " cmd " --"))))
+     (setq str (concat str "\n") done (concat done "\n"))
      ;; set the new header and footer
      (ewoc-set-hf cvs-cookies
  		 str (concat "\n--------------------- "
*** pcvs-info.el	Thu May  4 15:29:51 2006
--- pcvs-info.el.NEW	Fri May  5 12:43:07 2006
*************** (defun cvs-fileinfo-pp (fileinfo)
*** 379,385 ****
  		      ;; or nothing
  		      "")))
  	   (format "%-11s %s %-11s %-11s %s"
! 		   side status type base file)))))))
  
  
  (defun cvs-fileinfo-update (fi fi-new)
--- 379,386 ----
  		      ;; or nothing
  		      "")))
  	   (format "%-11s %s %-11s %-11s %s"
! 		   side status type base file))))
!      "\n")))
  
  
  (defun cvs-fileinfo-update (fi fi-new)

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2006-05-13  5:08 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-05-05 12:03 PATCH: ewoc.el to permit node representation abutment Thien-Thi Nguyen
2006-05-05 13:20 ` Stefan Monnier
2006-05-05 21:43   ` ttn
2006-05-08  8:48     ` Thien-Thi Nguyen
2006-05-08 15:04       ` Stefan Monnier
2006-05-13  5:08         ` Thien-Thi Nguyen

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).