From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Daniel Colascione Newsgroups: gmane.emacs.bugs Subject: bug#8415: 23.3.50; Extensible Emacs Registers Date: Mon, 04 Apr 2011 15:27:25 -0700 Message-ID: <4D9A45CD.4030808@gmail.com> References: NNTP-Posting-Host: lo.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit X-Trace: dough.gmane.org 1301956654 8228 80.91.229.12 (4 Apr 2011 22:37:34 GMT) X-Complaints-To: usenet@dough.gmane.org NNTP-Posting-Date: Mon, 4 Apr 2011 22:37:34 +0000 (UTC) Cc: Davis Herring , 8415@debbugs.gnu.org, Leo To: Stefan Monnier Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Tue Apr 05 00:37:30 2011 Return-path: Envelope-to: geb-bug-gnu-emacs@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 1Q6sOl-0002Tz-4g for geb-bug-gnu-emacs@m.gmane.org; Tue, 05 Apr 2011 00:37:27 +0200 Original-Received: from localhost ([127.0.0.1]:48261 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Q6sOk-0007Id-2i for geb-bug-gnu-emacs@m.gmane.org; Mon, 04 Apr 2011 18:37:26 -0400 Original-Received: from [140.186.70.92] (port=39285 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Q6sOa-0007DQ-W2 for bug-gnu-emacs@gnu.org; Mon, 04 Apr 2011 18:37:18 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Q6sOZ-0004XU-9m for bug-gnu-emacs@gnu.org; Mon, 04 Apr 2011 18:37:16 -0400 Original-Received: from debbugs.gnu.org ([140.186.70.43]:57462) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Q6sOZ-0004XL-7k for bug-gnu-emacs@gnu.org; Mon, 04 Apr 2011 18:37:15 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.69) (envelope-from ) id 1Q6sFd-0005r5-ND; Mon, 04 Apr 2011 18:28:01 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Daniel Colascione Original-Sender: debbugs-submit-bounces@debbugs.gnu.org Resent-To: owner@debbugs.gnu.org Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Mon, 04 Apr 2011 22:28:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 8415 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch Original-Received: via spool by 8415-submit@debbugs.gnu.org id=B8415.130195605922479 (code B ref 8415); Mon, 04 Apr 2011 22:28:01 +0000 Original-Received: (at 8415) by debbugs.gnu.org; 4 Apr 2011 22:27:39 +0000 Original-Received: from localhost ([127.0.0.1] helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1Q6sFG-0005qV-Mr for submit@debbugs.gnu.org; Mon, 04 Apr 2011 18:27:39 -0400 Original-Received: from mail-pv0-f172.google.com ([74.125.83.172]) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1Q6sFE-0005qI-8C for 8415@debbugs.gnu.org; Mon, 04 Apr 2011 18:27:37 -0400 Original-Received: by pvh1 with SMTP id 1so788080pvh.3 for <8415@debbugs.gnu.org>; Mon, 04 Apr 2011 15:27:30 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:message-id:date:from:user-agent:mime-version:to :cc:subject:references:in-reply-to:content-type :content-transfer-encoding; bh=XZtYl4YX/hsGByxdtg2NHYNDYhLff/tdVJNTiJh+Pn8=; b=S/a1FBQlptRNGMKaZWbj+WklkkVY3hDsrlYsMttHd3XIb/+2UVZpCRNsDMSNiSQLBc z9b1cRC/SRZoAnhelaBmdN1uJZ82jI+iLmmDG3A/T7Y61SXFe/MJBXPoFU2eg7Ggxfux 36Ct3zNkn6YJVCicbr48YjfhlqJZcwJ23K/mI= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=message-id:date:from:user-agent:mime-version:to:cc:subject :references:in-reply-to:content-type:content-transfer-encoding; b=SCGfh89pnq+jtCBGRqw5wBhvY8NFVJEYdfmdl2jO+h846Q8mn45DQgdy4Qw6CxaxY6 /iT16PB/e43+eXLAkwIxApNyiMEZ1Ld199WqsKh91SGpeEOkSkqwXlt4tQyCMHqji8Bn udf7TODPcxfuwN3b4j/dHGsRxctCiKIJNW/TU= Original-Received: by 10.142.250.20 with SMTP id x20mr6598799wfh.391.1301956050187; Mon, 04 Apr 2011 15:27:30 -0700 (PDT) Original-Received: from [0.0.0.0] (c-67-183-23-114.hsd1.wa.comcast.net [67.183.23.114]) by mx.google.com with ESMTPS id n4sm8115947wfl.14.2011.04.04.15.27.27 (version=SSLv3 cipher=OTHER); Mon, 04 Apr 2011 15:27:28 -0700 (PDT) User-Agent: Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.2.15) Gecko/20110303 Thunderbird/3.1.9 In-Reply-To: X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.11 Precedence: list Resent-Date: Mon, 04 Apr 2011 18:28:01 -0400 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6 (newer, 3) X-Received-From: 140.186.70.43 X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Original-Sender: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.bugs:45631 Archived-At: On 4/4/2011 3:19 PM, Stefan Monnier wrote: >>> A more backward-compatible change would be to not use register-structs >>> for pre-existing cases (i.e. markers, strings, lists of string, and >>> win-confs). I.e. only add register structs as a new accepted kind >>> of value (and move `name' out of the struct). >>> The patch would most likely be a lot smaller. > >> The original register.el is very inflexible and does its work mostly by >> guess because it misses the best moment to decide how to >> jump/insert/print a register i.e. at the time of creating it. > > AFAICT, the code currently doesn't guess: the different kinds of values > are mutually exclusive. So the moment at which they decide which > code to use doesn't matter because it'll give the same answer (tho > as you point out there are errors in this code currently because it's > dispersed). Polymorphism-via-typecase is delicate at best no matter what language we're working in. >> So we will have to make almost all values a struct anyway to fix bugs >> like this. > > Yes, all new types will use register structs. That's not a problem. > And you can even later-on de-support old types and have them go through > register structs as well. What's wrong with getting it over with now? >> As I said in another post, subsequent to this patch I will break down >> jump-to-register, describe-register-1, insert-register to take advantage >> of this new implementation. > > That's good. But I'd rather you break backward compatibility at *that* > point rather than right from the start. > I.e. start with a patch like the one below. Of course, instead of > register structs, you can use functions (like we do for completion > tables) as in: I'd still prefer Leo's approach here. Accessing register values directly wasn't common anyway, so the change shouldn't affect user code. If we're going to change the code, then IMHO it's better to start with a clean, orthogonal design where both existing cases and extensions use the same polymorphic system. All other things being equal, it's better to have one code path than two. I'd also slightly prefer Leo's structure approach to the dispatcher-function one below. Using separate struct fields leads to register-value-creating code more explicitly showing which operations are supported, and it also allows the register operation code to do something consistent when a particular register doesn't support some particular operation. Under the dispatcher function approach, the common register code has no idea whether a register value is going to do something intelligent with the given operation. > > === modified file 'lisp/register.el' > --- lisp/register.el 2011-01-25 04:08:28 +0000 > +++ lisp/register.el 2011-04-04 22:16:56 +0000 > @@ -52,7 +52,10 @@ > > (defvar register-alist nil > "Alist of elements (NAME . CONTENTS), one for each Emacs register. > -NAME is a character (a number). CONTENTS is a string, number, marker or list. > +NAME is a character (a number). CONTENTS can take various forms: > +A function that takes one argument (the action to perform). > + The action can be `print', `insert', or `jump'. Any action it does not > + understand should result in signalling an error. > A list of strings represents a rectangle. > A list of the form (file . FILE-NAME) represents the file named FILE-NAME. > A list of the form (file-query FILE-NAME POSITION) represents > @@ -120,6 +123,7 @@ > (interactive "cJump to register: \nP") > (let ((val (get-register register))) > (cond > + ((functionp val) (funcall val 'jump)) > ((and (consp val) (frame-configuration-p (car val))) > (set-frame-configuration (car val) (not delete)) > (goto-char (cadr val))) > @@ -209,6 +213,7 @@ > (princ " contains ") > (let ((val (get-register register))) > (cond > + ((functionp val) (funcall val 'print)) > ((numberp val) > (princ val)) > > @@ -285,6 +290,7 @@ > (push-mark) > (let ((val (get-register register))) > (cond > + ((functionp val) (funcall val 'insert)) > ((consp val) > (insert-rectangle val)) > ((stringp val) > > > -- Stefan > > > === modified file 'lisp/register.el' > --- lisp/register.el 2011-01-25 04:08:28 +0000 > +++ lisp/register.el 2011-04-04 22:10:11 +0000 > @@ -52,7 +52,8 @@ > > (defvar register-alist nil > "Alist of elements (NAME . CONTENTS), one for each Emacs register. > -NAME is a character (a number). CONTENTS is a string, number, marker or list. > +NAME is a character (a number). CONTENTS can take various forms: > +A `register' structure, made with `register-make'. > A list of strings represents a rectangle. > A list of the form (file . FILE-NAME) represents the file named FILE-NAME. > A list of the form (file-query FILE-NAME POSITION) represents > @@ -63,6 +64,18 @@ > A list of the form (FRAME-CONFIGURATION POSITION) > represents a saved frame configuration plus a saved value of point.") > > +(eval-when-compile (require 'cl)) > + > +(defstruct > + (register (:constructor nil) > + (:constructor register-make (value&key print-func > + jump-func insert-func)) > + (:copier nil)) > + (value nil :read-only t) > + (print-func nil :read-only t) > + (jump-func nil :read-only t) > + (insert-func nil :read-only t)) > + > (defun get-register (register) > "Return contents of Emacs register named REGISTER, or nil if none." > (cdr (assq register register-alist))) > @@ -120,6 +133,7 @@ > (interactive "cJump to register: \nP") > (let ((val (get-register register))) > (cond > + ((register-p val) (funcall (register-jump-func val) val)) > ((and (consp val) (frame-configuration-p (car val))) > (set-frame-configuration (car val) (not delete)) > (goto-char (cadr val))) > @@ -149,6 +163,7 @@ > > (defun register-swap-out () > "Turn markers into file-query references when a buffer is killed." > + ;; FIXME: Let register structures hook here as well. > (and buffer-file-name > (dolist (elem register-alist) > (and (markerp (cdr elem)) > @@ -177,6 +192,7 @@ > (defun increment-register (number register) > "Add NUMBER to the contents of register REGISTER. > Interactively, NUMBER is the prefix arg." > + ;; FIXME: Let register structures hook here as well. > (interactive "p\ncIncrement register: ") > (or (numberp (get-register register)) > (error "Register does not contain a number")) > @@ -209,6 +225,7 @@ > (princ " contains ") > (let ((val (get-register register))) > (cond > + ((register-p val) (funcall (register-print-func val) val)) > ((numberp val) > (princ val)) > > @@ -285,6 +302,7 @@ > (push-mark) > (let ((val (get-register register))) > (cond > + ((register-p val) (funcall (register-insert-func val) val)) > ((consp val) > (insert-rectangle val)) > ((stringp val) > @@ -315,6 +333,7 @@ > With prefix arg, delete as well. > Called from program, takes four args: REGISTER, START, END and DELETE-FLAG. > START and END are buffer positions indicating what to append." > + ;; FIXME: Let register structures hook here as well? > (interactive "cAppend to register: \nr\nP") > (let ((reg (get-register register)) > (text (filter-buffer-substring start end))) > @@ -329,6 +348,7 @@ > With prefix arg, delete as well. > Called from program, takes four args: REGISTER, START, END and DELETE-FLAG. > START and END are buffer positions indicating what to prepend." > + ;; FIXME: Let register structures hook here as well? > (interactive "cPrepend to register: \nr\nP") > (let ((reg (get-register register)) > (text (filter-buffer-substring start end))) >