RFC: Backwards-compatible change to tq.el

RFC: Backwards-compatible change to tq.el

[Michael Olson]

[-- Attachment #1.1: Type: text/plain, Size: 391 bytes --]

While looking at tq.el, I found that it interacts poorly with
processes that cannot receive input while sending output.  The
following patch will allow developers of new programs to fix this, but
retains backwards compatibility with older programs that use tq.el (if
any exist -- apparently no program that comes with Emacs uses tq.el).

If this patch receives approval, I will install it.


[-- Attachment #1.2: Type: application/pgp-signature, Size: 190 bytes --]

[-- Attachment #2: tq.el.patch --]
[-- Type: text/plain, Size: 3893 bytes --]

--- orig/lisp/ChangeLog
+++ mod/lisp/ChangeLog
@@ -1,3 +1,15 @@
+2006-04-11  Michael Olson  <mwolson@gnu.org>
+
+	* emacs-lisp/tq.el (tq-queue-head-question): New accessor
+	function.
+	(tq-queue-head-regexp, tq-queue-head-closure, tq-queue-head-fn):
+	Update for modified queue structure.
+	(tq-queue-add): Accept `question' argument.
+	(tq-queue-pop): If a question is pending, send it.
+	(tq-enqueue): Accept new optional argument `delay-question'.  If
+	this is non-nil, and at least one other question is pending a
+	response, queue the question rather than sending it immediately.
+
 2006-04-10  Bill Wohler  <wohler@newt.com>
 
 	* custom.el (defcustom, custom-handle-keyword): Add


--- orig/lisp/emacs-lisp/tq.el
+++ mod/lisp/emacs-lisp/tq.el
@@ -32,9 +32,9 @@
 ;; handler functions
 
 ;; Our basic structure is the queue/process/buffer triple.  Each entry
-;; of the queue is a regexp/closure/function triple.  We buffer
-;; bytes from the process until we see the regexp at the head of the
-;; queue.  Then we call the function with the closure and the
+;; of the queue is a question/regexp/closure/function quadruple.  We
+;; buffer bytes from the process until we see the regexp at the head
+;; of the queue.  Then we call the function with the closure and the
 ;; collected bytes.
 
 ;;; Code:
@@ -55,32 +55,45 @@
     tq))
 
 ;;; accessors
-(defun tq-queue   (tq) (car tq))
-(defun tq-process (tq) (car (cdr tq)))
-(defun tq-buffer  (tq) (cdr (cdr tq)))
+(defun tq-queue               (tq) (car tq))
+(defun tq-process             (tq) (car (cdr tq)))
+(defun tq-buffer              (tq) (cdr (cdr tq)))
+(defun tq-queue-head-question (tq) (car (car (tq-queue tq))))
+(defun tq-queue-head-regexp   (tq) (car (cdr (car (tq-queue tq)))))
+(defun tq-queue-head-closure  (tq) (car (cdr (cdr (car (tq-queue tq))))))
+(defun tq-queue-head-fn       (tq) (cdr (cdr (cdr (car (tq-queue tq))))))
 
-(defun tq-queue-add (tq re closure fn)
+(defun tq-queue-empty         (tq) (not (tq-queue tq)))
+
+(defun tq-queue-add (tq question re closure fn)
   (setcar tq (nconc (tq-queue tq)
-		    (cons (cons re (cons closure fn)) nil)))
+		    (cons (cons question (cons re (cons closure fn))) nil)))
   'ok)
 
-(defun tq-queue-head-regexp  (tq) (car (car (tq-queue tq))))
-(defun tq-queue-head-fn      (tq) (cdr (cdr (car (tq-queue tq)))))
-(defun tq-queue-head-closure (tq) (car (cdr (car (tq-queue tq)))))
-(defun tq-queue-empty        (tq) (not (tq-queue tq)))
-(defun tq-queue-pop          (tq) (setcar tq (cdr (car tq))) (null (car tq)))
-
+(defun tq-queue-pop (tq)
+  (setcar tq (cdr (car tq)))
+  (let ((question (tq-queue-head-question tq)))
+    (when question
+      (process-send-string (tq-process tq) question)))
+  (null (car tq)))
 
 ;;; must add to queue before sending!
-(defun tq-enqueue (tq question regexp closure fn)
+(defun tq-enqueue (tq question regexp closure fn &optional delay-question)
   "Add a transaction to transaction queue TQ.
 This sends the string QUESTION to the process that TQ communicates with.
 When the corresponding answer comes back, we call FN
 with two arguments: CLOSURE, and the answer to the question.
 REGEXP is a regular expression to match the entire answer;
-that's how we tell where the answer ends."
-  (tq-queue-add tq regexp closure fn)
-  (process-send-string (tq-process tq) question))
+that's how we tell where the answer ends.
+
+If DELAY-QUESTION is non-nil, delay sending this question until
+the process has finished replying to any previous questions.
+This produces more reliable results with some processes."
+  (let ((sendp (or (not delay-question)
+		   (not (tq-queue-head-question tq)))))
+    (tq-queue-add tq (and delay-question question) regexp closure fn)
+    (when sendp
+      (process-send-string (tq-process tq) question))))
 
 (defun tq-close (tq)
   "Shut down transaction queue TQ, terminating the process."





[-- Attachment #3: Type: text/plain, Size: 262 bytes --]


-- 
Michael Olson -- FSF Associate Member #652 -- http://www.mwolson.org/
Interests: Emacs Lisp, text markup, protocols -- Muse, Planner, ERC, EMMS
  /` |\ | | | IRC: mwolson on freenode.net: #hcoop, #muse, #PurdueLUG
 |_] | \| |_| Jabber: mwolson_at_hcoop.net

[-- Attachment #4: Type: text/plain, Size: 142 bytes --]

_______________________________________________
Emacs-devel mailing list
Emacs-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/emacs-devel

Re: RFC: Backwards-compatible change to tq.el

[Stuart D. Herring]

[Michael: sorry to double-send; I wanted to include the list.]

> While looking at tq.el, I found that it interacts poorly with
> processes that cannot receive input while sending output.  The
> following patch will allow developers of new programs to fix this, but
retains backwards compatibility with older programs that use tq.el (if
any exist -- apparently no program that comes with Emacs uses tq.el).

I don't know much about tq.el, but I'm leery of the line

(tq-queue-add tq (and delay-question question) regexp closure fn)

-- it seems to me that you want to include 'question' if and only if you
aren't already sending it (because tq-queue-pop will definitely send it if
you include it).  So shouldn't it be (unless sendp question) rather than
(and delay-question question)?

Davis

-- 
This product is sold by volume, not by mass.  If it appears too dense or
too sparse, it is because mass-energy conversion has occurred during
shipping.

Re: RFC: Backwards-compatible change to tq.el

[Michael Olson]

[-- Attachment #1.1: Type: text/plain, Size: 567 bytes --]

"Stuart D. Herring" <herring@lanl.gov> writes:

> I don't know much about tq.el, but I'm leery of the line
>
> (tq-queue-add tq (and delay-question question) regexp closure fn)
>
> -- it seems to me that you want to include 'question' if and only if
> you aren't already sending it (because tq-queue-pop will definitely
> send it if you include it).  So shouldn't it be (unless sendp
> question) rather than (and delay-question question)?

Good catch.  I've fixed this in the attached patch.  I've also revised
the comments to include additional useful information.


[-- Attachment #1.2: Type: application/pgp-signature, Size: 190 bytes --]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: tq.el.patch --]
[-- Type: text/x-patch, Size: 5260 bytes --]

--- orig/lisp/ChangeLog
+++ mod/lisp/ChangeLog
@@ -1,3 +1,15 @@
+2006-04-11  Michael Olson  <mwolson@gnu.org>
+
+	* emacs-lisp/tq.el (tq-queue-head-question): New accessor
+	function.
+	(tq-queue-head-regexp, tq-queue-head-closure, tq-queue-head-fn):
+	Update for modified queue structure.
+	(tq-queue-add): Accept `question' argument.
+	(tq-queue-pop): If a question is pending, send it.
+	(tq-enqueue): Accept new optional argument `delay-question'.  If
+	this is non-nil, and at least one other question is pending a
+	response, queue the question rather than sending it immediately.
+
 2006-04-10  Bill Wohler  <wohler@newt.com>
 
 	* custom.el (defcustom, custom-handle-keyword): Add


--- orig/lisp/emacs-lisp/tq.el
+++ mod/lisp/emacs-lisp/tq.el
@@ -27,18 +27,51 @@
 
 ;;; Commentary:
 
-;; manages receiving a stream asynchronously,
-;; parsing it into transactions, and then calling
-;; handler functions
+;; This file manages receiving a stream either asynchronously or
+;; (optionally) synchronously, parsing it into transactions, and then
+;; calling the associated handler function upon the completion of each
+;; transaction.
 
 ;; Our basic structure is the queue/process/buffer triple.  Each entry
-;; of the queue is a regexp/closure/function triple.  We buffer
-;; bytes from the process until we see the regexp at the head of the
-;; queue.  Then we call the function with the closure and the
-;; collected bytes.
+;; of the queue part is a list of question, regexp, closure, and
+;; function that is consed to the last element.
+
+;; A transaction queue may be created by calling `tq-create'.
+
+;; A request may be added to the queue by calling `tq-enqueue'.  If
+;; the `delay-question' argument is non-nil, we will wait to send the
+;; question to the process until it has finished sending other input.
+;; Otherwise, once a request is enqueued, we send the given question
+;; immediately to the process.
+
+;; We then buffer bytes from the process until we see the regexp that
+;; was provided in the call to `tq-enqueue'.  Then we call the
+;; provided function with the closure and the collected bytes.  If we
+;; have indicated that the question from the next transaction was not
+;; sent immediately, send it at this point, awaiting the response.
 
 ;;; Code:
 
+;;; Accessors
+
+;; This part looks like (queue . (process . buffer))
+(defun tq-queue               (tq) (car tq))
+(defun tq-process             (tq) (car (cdr tq)))
+(defun tq-buffer              (tq) (cdr (cdr tq)))
+
+;; The structure of `queue' is as follows
+;; ((question regexp closure . fn)
+;;  <other queue entries>)
+(defun tq-queue-head-question (tq) (car (car (tq-queue tq))))
+(defun tq-queue-head-regexp   (tq) (car (cdr (car (tq-queue tq)))))
+(defun tq-queue-head-closure  (tq) (car (cdr (cdr (car (tq-queue tq))))))
+(defun tq-queue-head-fn       (tq) (cdr (cdr (cdr (car (tq-queue tq))))))
+
+;; Determine whether queue is empty
+(defun tq-queue-empty         (tq) (not (tq-queue tq)))
+
+;;; Core functionality
+
 ;;;###autoload
 (defun tq-create (process)
   "Create and return a transaction queue communicating with PROCESS.
@@ -54,33 +87,34 @@
 			   (tq-filter ',tq string)))
     tq))
 
-;;; accessors
-(defun tq-queue   (tq) (car tq))
-(defun tq-process (tq) (car (cdr tq)))
-(defun tq-buffer  (tq) (cdr (cdr tq)))
-
-(defun tq-queue-add (tq re closure fn)
+(defun tq-queue-add (tq question re closure fn)
   (setcar tq (nconc (tq-queue tq)
-		    (cons (cons re (cons closure fn)) nil)))
+		    (cons (cons question (cons re (cons closure fn))) nil)))
   'ok)
 
-(defun tq-queue-head-regexp  (tq) (car (car (tq-queue tq))))
-(defun tq-queue-head-fn      (tq) (cdr (cdr (car (tq-queue tq)))))
-(defun tq-queue-head-closure (tq) (car (cdr (car (tq-queue tq)))))
-(defun tq-queue-empty        (tq) (not (tq-queue tq)))
-(defun tq-queue-pop          (tq) (setcar tq (cdr (car tq))) (null (car tq)))
+(defun tq-queue-pop (tq)
+  (setcar tq (cdr (car tq)))
+  (let ((question (tq-queue-head-question tq)))
+    (when question
+      (process-send-string (tq-process tq) question)))
+  (null (car tq)))
 
-
-;;; must add to queue before sending!
-(defun tq-enqueue (tq question regexp closure fn)
+(defun tq-enqueue (tq question regexp closure fn &optional delay-question)
   "Add a transaction to transaction queue TQ.
 This sends the string QUESTION to the process that TQ communicates with.
 When the corresponding answer comes back, we call FN
 with two arguments: CLOSURE, and the answer to the question.
 REGEXP is a regular expression to match the entire answer;
-that's how we tell where the answer ends."
-  (tq-queue-add tq regexp closure fn)
-  (process-send-string (tq-process tq) question))
+that's how we tell where the answer ends.
+
+If DELAY-QUESTION is non-nil, delay sending this question until
+the process has finished replying to any previous questions.
+This produces more reliable results with some processes."
+  (let ((sendp (or (not delay-question)
+		   (not (tq-queue-head-question tq)))))
+    (tq-queue-add tq (unless sendp question) regexp closure fn)
+    (when sendp
+      (process-send-string (tq-process tq) question))))
 
 (defun tq-close (tq)
   "Shut down transaction queue TQ, terminating the process."





[-- Attachment #3: Type: text/plain, Size: 262 bytes --]


-- 
Michael Olson -- FSF Associate Member #652 -- http://www.mwolson.org/
Interests: Emacs Lisp, text markup, protocols -- Muse, Planner, ERC, EMMS
  /` |\ | | | IRC: mwolson on freenode.net: #hcoop, #muse, #PurdueLUG
 |_] | \| |_| Jabber: mwolson_at_hcoop.net

[-- Attachment #4: Type: text/plain, Size: 142 bytes --]

_______________________________________________
Emacs-devel mailing list
Emacs-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/emacs-devel

Re: RFC: Backwards-compatible change to tq.el

[Richard Stallman]

    +;; The structure of `queue' is as follows
    +;; ((question regexp closure . fn)
    +;;  <other queue entries>)

That shows the data's structure, but the structure
is just the first part of documenting it.
It's necessary to explain what each of the data _means_.

Could you please add that too?

Re: RFC: Backwards-compatible change to tq.el

[Michael Olson]

[-- Attachment #1.1: Type: text/plain, Size: 409 bytes --]

Richard Stallman <rms@gnu.org> writes:

>     +;; The structure of `queue' is as follows
>     +;; ((question regexp closure . fn)
>     +;;  <other queue entries>)
>
> That shows the data's structure, but the structure
> is just the first part of documenting it.
> It's necessary to explain what each of the data _means_.

Here's another version with additional documentation on each part of
the structure.


[-- Attachment #1.2: Type: application/pgp-signature, Size: 190 bytes --]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: tq.el.patch --]
[-- Type: text/x-patch, Size: 5650 bytes --]

--- orig/lisp/ChangeLog
+++ mod/lisp/ChangeLog
@@ -1,3 +1,15 @@
+2006-04-11  Michael Olson  <mwolson@gnu.org>
+
+	* emacs-lisp/tq.el (tq-queue-head-question): New accessor
+	function.
+	(tq-queue-head-regexp, tq-queue-head-closure, tq-queue-head-fn):
+	Update for modified queue structure.
+	(tq-queue-add): Accept `question' argument.
+	(tq-queue-pop): If a question is pending, send it.
+	(tq-enqueue): Accept new optional argument `delay-question'.  If
+	this is non-nil, and at least one other question is pending a
+	response, queue the question rather than sending it immediately.
+
 2006-04-10  Bill Wohler  <wohler@newt.com>
 
 	* custom.el (defcustom, custom-handle-keyword): Add


--- orig/lisp/emacs-lisp/tq.el
+++ mod/lisp/emacs-lisp/tq.el
@@ -27,18 +27,56 @@
 
 ;;; Commentary:
 
-;; manages receiving a stream asynchronously,
-;; parsing it into transactions, and then calling
-;; handler functions
+;; This file manages receiving a stream asynchronously, parsing it
+;; into transactions, and then calling the associated handler function
+;; upon the completion of each transaction.
 
 ;; Our basic structure is the queue/process/buffer triple.  Each entry
-;; of the queue is a regexp/closure/function triple.  We buffer
-;; bytes from the process until we see the regexp at the head of the
-;; queue.  Then we call the function with the closure and the
-;; collected bytes.
+;; of the queue part is a list of question, regexp, closure, and
+;; function that is consed to the last element.
+
+;; A transaction queue may be created by calling `tq-create'.
+
+;; A request may be added to the queue by calling `tq-enqueue'.  If
+;; the `delay-question' argument is non-nil, we will wait to send the
+;; question to the process until it has finished sending other input.
+;; Otherwise, once a request is enqueued, we send the given question
+;; immediately to the process.
+
+;; We then buffer bytes from the process until we see the regexp that
+;; was provided in the call to `tq-enqueue'.  Then we call the
+;; provided function with the closure and the collected bytes.  If we
+;; have indicated that the question from the next transaction was not
+;; sent immediately, send it at this point, awaiting the response.
 
 ;;; Code:
 
+;;; Accessors
+
+;; This part looks like (queue . (process . buffer))
+(defun tq-queue               (tq) (car tq))
+(defun tq-process             (tq) (car (cdr tq)))
+(defun tq-buffer              (tq) (cdr (cdr tq)))
+
+;; The structure of `queue' is as follows
+;; ((question regexp closure . fn)
+;;  <other queue entries>)
+;; question: string to send to the process
+(defun tq-queue-head-question (tq) (car (car (tq-queue tq))))
+;; regexp: regular expression that matches the end of a response from
+;; the process
+(defun tq-queue-head-regexp   (tq) (car (cdr (car (tq-queue tq)))))
+;; closure: additional data to pass to function
+(defun tq-queue-head-closure  (tq) (car (cdr (cdr (car (tq-queue tq))))))
+;; fn: function to call upon receiving a complete response from the
+;; process
+(defun tq-queue-head-fn       (tq) (cdr (cdr (cdr (car (tq-queue tq))))))
+
+;; Determine whether queue is empty
+(defun tq-queue-empty         (tq) (not (tq-queue tq)))
+
+;;; Core functionality
+
 ;;;###autoload
 (defun tq-create (process)
   "Create and return a transaction queue communicating with PROCESS.
@@ -54,33 +92,37 @@
 			   (tq-filter ',tq string)))
     tq))
 
-;;; accessors
-(defun tq-queue   (tq) (car tq))
-(defun tq-process (tq) (car (cdr tq)))
-(defun tq-buffer  (tq) (cdr (cdr tq)))
-
-(defun tq-queue-add (tq re closure fn)
+(defun tq-queue-add (tq question re closure fn)
   (setcar tq (nconc (tq-queue tq)
-		    (cons (cons re (cons closure fn)) nil)))
+		    (cons (cons question (cons re (cons closure fn))) nil)))
   'ok)
 
-(defun tq-queue-head-regexp  (tq) (car (car (tq-queue tq))))
-(defun tq-queue-head-fn      (tq) (cdr (cdr (car (tq-queue tq)))))
-(defun tq-queue-head-closure (tq) (car (cdr (car (tq-queue tq)))))
-(defun tq-queue-empty        (tq) (not (tq-queue tq)))
-(defun tq-queue-pop          (tq) (setcar tq (cdr (car tq))) (null (car tq)))
-
+(defun tq-queue-pop (tq)
+  (setcar tq (cdr (car tq)))
+  (let ((question (tq-queue-head-question tq)))
+    (when question
+      (process-send-string (tq-process tq) question)))
+  (null (car tq)))
 
-;;; must add to queue before sending!
-(defun tq-enqueue (tq question regexp closure fn)
+(defun tq-enqueue (tq question regexp closure fn &optional delay-question)
   "Add a transaction to transaction queue TQ.
 This sends the string QUESTION to the process that TQ communicates with.
-When the corresponding answer comes back, we call FN
-with two arguments: CLOSURE, and the answer to the question.
+
+When the corresponding answer comes back, we call FN with two
+arguments: CLOSURE, which may contain additional data that FN
+needs, and the answer to the question.
+
 REGEXP is a regular expression to match the entire answer;
-that's how we tell where the answer ends."
-  (tq-queue-add tq regexp closure fn)
-  (process-send-string (tq-process tq) question))
+that's how we tell where the answer ends.
+
+If DELAY-QUESTION is non-nil, delay sending this question until
+the process has finished replying to any previous questions.
+This produces more reliable results with some processes."
+  (let ((sendp (or (not delay-question)
+		   (not (tq-queue-head-question tq)))))
+    (tq-queue-add tq (unless sendp question) regexp closure fn)
+    (when sendp
+      (process-send-string (tq-process tq) question))))
 
 (defun tq-close (tq)
   "Shut down transaction queue TQ, terminating the process."





[-- Attachment #3: Type: text/plain, Size: 262 bytes --]


-- 
Michael Olson -- FSF Associate Member #652 -- http://www.mwolson.org/
Interests: Emacs Lisp, text markup, protocols -- Muse, Planner, ERC, EMMS
  /` |\ | | | IRC: mwolson on freenode.net: #hcoop, #muse, #PurdueLUG
 |_] | \| |_| Jabber: mwolson_at_hcoop.net

[-- Attachment #4: Type: text/plain, Size: 142 bytes --]

_______________________________________________
Emacs-devel mailing list
Emacs-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/emacs-devel

Re: RFC: Backwards-compatible change to tq.el

[Richard Stallman]

It looks good now.  Would someone please install it?

Re: RFC: Backwards-compatible change to tq.el

[Michael Olson]

[-- Attachment #1.1: Type: text/plain, Size: 455 bytes --]

Richard Stallman <rms@gnu.org> writes:

> It looks good now.  Would someone please install it?

Done -- the corresponding Arch commit is
emacs@sv.gnu.org/emacs--devo--0--patch-220.

-- 
Michael Olson -- FSF Associate Member #652 -- http://www.mwolson.org/
Interests: Emacs Lisp, text markup, protocols -- Muse, Planner, ERC, EMMS
  /` |\ | | | IRC: mwolson on freenode.net: #hcoop, #muse, #PurdueLUG
 |_] | \| |_| Jabber: mwolson_at_hcoop.net

[-- Attachment #1.2: Type: application/pgp-signature, Size: 190 bytes --]

[-- Attachment #2: Type: text/plain, Size: 142 bytes --]

_______________________________________________
Emacs-devel mailing list
Emacs-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/emacs-devel