From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Stefan Monnier Newsgroups: gmane.emacs.devel Subject: Re: ewoc patch Date: Sat, 21 Aug 2010 12:52:27 +0200 Message-ID: References: <08698ba442eec12008bede6987588958.squirrel@mail.panix.com> NNTP-Posting-Host: lo.gmane.org Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" X-Trace: dough.gmane.org 1282388043 23874 80.91.229.12 (21 Aug 2010 10:54:03 GMT) X-Complaints-To: usenet@dough.gmane.org NNTP-Posting-Date: Sat, 21 Aug 2010 10:54:03 +0000 (UTC) Cc: emacs-devel@gnu.org To: "Tom Breton \(Tehom\)" Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Sat Aug 21 12:54:01 2010 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([199.232.76.165]) by lo.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1OmlhO-0002fs-Gy for ged-emacs-devel@m.gmane.org; Sat, 21 Aug 2010 12:54:01 +0200 Original-Received: from localhost ([127.0.0.1]:52984 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1OmlhI-0006Yz-HY for ged-emacs-devel@m.gmane.org; Sat, 21 Aug 2010 06:53:13 -0400 Original-Received: from [140.186.70.92] (port=58656 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Omlh1-0006Tj-FB for emacs-devel@gnu.org; Sat, 21 Aug 2010 06:53:08 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1Omlgz-0002rC-6m for emacs-devel@gnu.org; Sat, 21 Aug 2010 06:52:55 -0400 Original-Received: from impaqm3.telefonica.net ([213.4.138.3]:35907) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1Omlgy-0002qn-N7 for emacs-devel@gnu.org; Sat, 21 Aug 2010 06:52:53 -0400 Original-Received: from IMPmailhost2.adm.correo ([10.20.102.39]) by IMPaqm3.telefonica.net with bizsmtp id wynx1e0190r0BT63PysVvZ; Sat, 21 Aug 2010 12:52:29 +0200 Original-Received: from ceviche.home ([83.61.35.93]) by IMPmailhost2.adm.correo with BIZ IMP id wysU1e00820aCvn1iysUV8; Sat, 21 Aug 2010 12:52:29 +0200 X-Brightmail-Tracker: AAAAAA== X-TE-authinfo: authemail="monnier$movistar.es" |auth_email="monnier@movistar.es" X-TE-AcuTerraCos: auth_cuTerraCos="cosuitnetc01" Original-Received: by ceviche.home (Postfix, from userid 20848) id 283DF660E9; Sat, 21 Aug 2010 12:52:27 +0200 (CEST) In-Reply-To: (Tom Breton's message of "Thu, 10 Dec 2009 20:23:16 -0500") User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.0.50 (gnu/linux) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6 (newer, 3) X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Original-Sender: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.devel:128954 Archived-At: --=-=-= [ Sorry for the long delay. ] Could you remind me of the purpose of this change? Was it only to add a `separator' argument to ewoc? While, I'm here I noticed that your code did not follow our indentation and layout conventions, so I've fixed that in the attached patch (where I also replaced the ewoc-provides-variable-separator with a subfeature). Finally, regarding this comment: ;; A start-marker's insertion-type should already be `t', but some ;; callers want to be 100% sure it is, so this function exists. is it the case that it would be a bug if the insertion-type was changed to nil, or are there legitimate cases where the insertion-type is changed to nil (and if so, which are these)? I ask because if there's no legitimate case, then we should just `assert' it. Stefan >>>>> "Tom" == Tom Breton (Tehom) writes: >> Keep only `separator', but with the added twist that a non-string, >> non-nil argument means the empty string (aka mean "nosep"). That should >> preserve backward compatibility with a mostly clean API. > Done. >> I think you're worrying for no good reason. If you really want to >> handle version dependencies right, you don't want it in a Lisp variable >> but in a header understood and processed by some package manager (ELPA, >> for instace). Otherwise, you'll have to deal (one way or another) with >> runtime checks, and in most cases the calling package will simply not >> work with an older version. > Yes, I have to deal with runtime checks. > Anyways, I have replaced the version variable with a variable called > `ewoc-provides-variable-separator' that is bound just if ewoc provides > a variable separator. I hope that will satisfy. > Attached is > * new ewoc patch against 23.1 > * test file > Thank you for your help, Stefan. > Tom Breton (Tehom) --=-=-= Content-Type: text/x-diff Content-Disposition: inline; filename=ewoc.diff --- lisp/emacs-lisp/ewoc.el 2010-08-21 12:39:33.000000000 +0200 +++ ../trunk/lisp/emacs-lisp/ewoc.el 2010-08-21 12:49:41.000000000 +0200 @@ -139,7 +139,8 @@ (defstruct (ewoc (:constructor nil) - (:constructor ewoc--create (buffer pretty-printer dll separator)) + (:constructor ewoc--create (buffer pretty-printer dll + &optional separator)) (:conc-name ewoc--)) buffer pretty-printer header footer dll last-node hf-pp separator) @@ -167,12 +168,12 @@ node)) (defun ewoc--link-node-before (new-node node) - "Physically insert NEW-NODE before NODE." - (setf - (ewoc--node-left new-node) (ewoc--node-left node) - (ewoc--node-right new-node) node - (ewoc--node-right (ewoc--node-left node)) new-node - (ewoc--node-left node) new-node)) + "Physically insert NEW-NODE before NODE." + (setf + (ewoc--node-left new-node) (ewoc--node-left node) + (ewoc--node-right new-node) node + (ewoc--node-right (ewoc--node-left node)) new-node + (ewoc--node-left node) new-node)) (defun ewoc--insert-new-node (ewoc node data pretty-printer) "Insert before NODE a new node for DATA, displayed by PRETTY-PRINTER. @@ -181,112 +182,99 @@ NODE and leaving the new node's start there. Return the new node." (save-excursion (let ((elemnode (ewoc--node-create nil data))) - (ewoc--link-node-before elemnode node) - (goto-char (ewoc-location-safe node)) - (ewoc--print-node ewoc elemnode pretty-printer) + (ewoc--link-node-before elemnode node) + (goto-char (ewoc-location-safe node)) + (ewoc--print-node ewoc elemnode pretty-printer) elemnode))) (defun ewoc--refresh-node (pp node ewoc) "Redisplay the element represented by NODE using the pretty-printer PP." - (save-excursion - (goto-char (ewoc-location-safe node)) - (ewoc--delete-node-text node) - (ewoc--print-anew ewoc node pp))) + (save-excursion + (goto-char (ewoc-location-safe node)) + (ewoc--delete-node-text node) + (ewoc--print-anew ewoc node pp))) ;;; =========================================================================== ;;; Node location (defun ewoc--next-printed-node (node) - "Return the next non-empty node after NODE." - ;;This loop will terminate because we set at least one - ;;start-marker in the ewoc when creating it. - (do - ((node-after - (ewoc--node-right node) - (ewoc--node-right node-after))) - ( - (ewoc--node-start-marker node-after) - node-after))) - + "Return the next non-empty node after NODE." + ;; This loop will terminate because we set at least one + ;; start-marker in the ewoc when creating it. + (do ((node-after + (ewoc--node-right node) + (ewoc--node-right node-after))) + ((ewoc--node-start-marker node-after) + node-after))) + (defun ewoc--next-start-marker (node) - "Return the first start marker after NODE." - (ewoc--node-start-marker - (ewoc--next-printed-node node))) + "Return the first start marker after NODE." + (ewoc--node-start-marker + (ewoc--next-printed-node node))) (defun ewoc-location (node) - "Return the start location of NODE. + "Return the start location of NODE. If NODE is empty, return the start marker of the next non-empty node." - (or - (ewoc--node-start-marker node) + (or (ewoc--node-start-marker node) (ewoc--next-start-marker node))) -;;A start-marker's insertion-type should already be `t', but some -;;callers want to be 100% sure it is, so this function exists. +;; A start-marker's insertion-type should already be `t', but some +;; callers want to be 100% sure it is, so this function exists. (defun ewoc-location-safe (node) - "Get NODE's start location. + "Get NODE's start location. Also set the start-marker's insertion type to `t' so that it will stay after any text inserted at that point." - - (let - ((next-start-marker (ewoc-location node))) - (set-marker-insertion-type next-start-marker t) - next-start-marker)) + (let ((next-start-marker (ewoc-location node))) + (set-marker-insertion-type next-start-marker t) + next-start-marker)) ;;; =========================================================================== ;;; Printing and unprinting (defun ewoc--mark-node-empty (node) - "Mark NODE empty (but don't empty it, assume it was emptied) + "Mark NODE empty (but don't empty it, assume it was emptied) INTERNAL USE ONLY." - (let - ((start-marker (ewoc--node-start-marker node))) - (when start-marker - (set-marker start-marker nil) - (setf (ewoc--node-start-marker node) nil)))) + (let ((start-marker (ewoc--node-start-marker node))) + (when start-marker + (set-marker start-marker nil) + (setf (ewoc--node-start-marker node) nil)))) (defun ewoc--delete-node-text (node) - "Delete a node's text and mark it empty." - (let - ((inhibit-read-only t) - (m (ewoc--node-start-marker node))) - (when m - (delete-region m (ewoc--next-start-marker node)) - (ewoc--mark-node-empty node)))) + "Delete a node's text and mark it empty." + (let ((inhibit-read-only t) + (m (ewoc--node-start-marker node))) + (when m + (delete-region m (ewoc--next-start-marker node)) + (ewoc--mark-node-empty node)))) (defun ewoc--print-anew (ewoc node pp) - "Print a node that was erased but not marked empty." - (ewoc--mark-node-empty node) - (ewoc--print-node ewoc node pp)) + "Print a node that was erased but not marked empty." + (ewoc--mark-node-empty node) + (ewoc--print-node ewoc node pp)) + - (defun ewoc--print-node (ewoc node printer) - "Print NODE at point using PRINTER. + "Print NODE at point using PRINTER. Set NODE's start-marker accordingly." - ;;Only print if node is currently empty - (when (ewoc--node-start-marker node) - (error "ewoc--print-node called with a node that's already printed")) - - (let - ( - (start-pos (point)) - (inhibit-read-only t)) - - (funcall printer (ewoc-data node)) - (let - ((separator (ewoc--separator ewoc))) - (when separator (insert separator))) - - ;;Only set up this node as non-empty if it actually is - ;;non-empty. - (unless - (= start-pos (point)) - ;;Set its start-marker to the position we started - ;;printing from. - (setf - (ewoc--node-start-marker node) - (copy-marker start-pos t))))) + ;; Only print if node is currently empty. + (when (ewoc--node-start-marker node) + (error "ewoc--print-node called with a node that's already printed")) + + (let ((start-pos (point)) + (inhibit-read-only t)) + + (funcall printer (ewoc-data node)) + (let ((separator (ewoc--separator ewoc))) + (when separator (insert separator))) + + ;; Only set up this node as non-empty if it actually is non-empty. + (unless (= start-pos (point)) + ;; Set its start-marker to the position we started printing from. + (setf + (ewoc--node-start-marker node) + (copy-marker start-pos t))))) ;;; =========================================================================== @@ -316,12 +304,12 @@ (setf (ewoc--node-left dummy-node) dummy-node) dummy-node)) (separator - (cond - ((null separator) "\n") - ((stringp separator) - (if (string= separator "") nil separator)) - ;;Non-nil, non-string argument means empty separator - (t nil))) + (cond + ((null separator) "\n") + ((stringp separator) + (if (string= separator "") nil separator)) + ;;Non-nil, non-string argument means empty separator + (t nil))) (new-ewoc (ewoc--create (current-buffer) pretty-printer dll @@ -426,15 +414,15 @@ (defalias 'ewoc-foreach 'ewoc-map) (defmacro ewoc-do (spec &rest body) - "Evaluate BODY repeatedly, with NAME bound successively to the data + "Evaluate BODY repeatedly, with NAME bound successively to the data of each element. The element will be refreshed if BODY returns non-nil." - (destructuring-bind (name ewoc-form) spec - `(progn - (ewoc-foreach - #'(lambda (,name) - ,@body) - ,ewoc-form)))) + (destructuring-bind (name ewoc-form) spec + `(progn + (ewoc-foreach + #'(lambda (,name) + ,@body) + ,ewoc-form)))) (defun ewoc-delete (ewoc &rest nodes) @@ -490,11 +478,11 @@ nil) ;; Before second elem? - ((< pos (ewoc-location (ewoc--node-nth dll 2))) + ((< pos (ewoc-location (ewoc--node-nth dll 2))) (ewoc--node-nth dll 1)) ;; After one-before-last elem? - ((>= pos (ewoc-location (ewoc--node-nth dll -2))) + ((>= pos (ewoc-location (ewoc--node-nth dll -2))) (ewoc--node-nth dll -2)) ;; We now know that pos is within a elem. @@ -529,16 +517,16 @@ (cond ;; Is pos after the guess? ((>= pos - (ewoc-location best-guess)) + (ewoc-location best-guess)) ;; Loop until we are exactly one node too far down... - (while (>= pos (ewoc-location best-guess)) + (while (>= pos (ewoc-location best-guess)) (setq best-guess (ewoc--node-next dll best-guess))) ;; ...and return the previous node. (ewoc--node-prev dll best-guess)) ;; Pos is before best-guess (t - (while (< pos (ewoc-location best-guess)) + (while (< pos (ewoc-location best-guess)) (setq best-guess (ewoc--node-prev dll best-guess))) best-guess))))))) @@ -559,7 +547,7 @@ ((node (ewoc-locate ewoc (point)))) (when node ;; If we were past the last element, first jump to it. - (when (>= (point) (ewoc-location (ewoc--node-right node))) + (when (>= (point) (ewoc-location (ewoc--node-right node))) (setq arg (1- arg))) (while (and node (> arg 0)) (setq arg (1- arg)) @@ -585,7 +573,7 @@ (defun ewoc-goto-node (ewoc node) "Move point to NODE in EWOC." (ewoc--set-buffer-bind-dll ewoc - (goto-char (ewoc-location node)) + (goto-char (ewoc-location node)) (if goal-column (move-to-column goal-column)) (setf (ewoc--last-node ewoc) node))) @@ -598,14 +586,14 @@ (ewoc--set-buffer-bind-dll-let* ewoc ((footer (ewoc--footer ewoc))) (let ((inhibit-read-only t) - (first-node (ewoc--node-nth dll 1))) + (first-node (ewoc--node-nth dll 1))) (delete-region (ewoc-location first-node) (ewoc-location footer)) (goto-char (ewoc-location footer)) (let ((pp (ewoc--pretty-printer ewoc)) (node first-node)) (while (not (eq node footer)) - (ewoc--print-anew ewoc node pp) + (ewoc--print-anew ewoc node pp) (setq node (ewoc--node-next dll node))))) (set-marker (ewoc-location footer) (point)))) @@ -655,8 +643,7 @@ (ewoc--refresh-node hf-pp foot ewoc)))) -(defconst ewoc-provides-variable-separator t) -(provide 'ewoc) +(provide 'ewoc '(separator)) ;; Local Variables: ;; eval: (put 'ewoc--set-buffer-bind-dll 'lisp-indent-hook 1) --=-=-=--