all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* accept-process-output incompatible change in CVS Emacs: patch included
@ 2007-05-04  8:56 Per Cederqvist
  2007-05-04 22:33 ` Kim F. Storm
  0 siblings, 1 reply; 6+ messages in thread
From: Per Cederqvist @ 2007-05-04  8:56 UTC (permalink / raw)
  To: bug-gnu-emacs

[-- Attachment #1: Type: text/plain, Size: 2086 bytes --]

This change in the upcoming Emacs 22 breaks some code:

> 2006-03-22  Kim F. Storm  <storm@cua.dk>
> 
> 	* process.c (Faccept_process_output): Fix to comply with lisp
> 	reference.  Change arg "timeout" to "seconds" and allow both
> 	integer and float value.  Change arg "timeout-msec" to "millisec"
> 	and interpret" as milliseconds rather than microseconds.  Fix doc
> 	string accordingly.

I enclose a patch that makes accept-process-output
backwards-compatible with Emacs 21.

One application I'm running now runs very slowly, since its carefully
tuned timeoutes are multiplied by 1000.

The third argument of the accept-process-output function used to be a
number of microseconds.  Apparently whoever wrote the GNU Emacs Lisp
Reference Manual mistook "msec" to mean milliseconds.  But the
built-in documentation in Emacs 21.3 clearly states that the argument
is in milliseconds:

> accept-process-output is a built-in function.
> (accept-process-output &optional PROCESS TIMEOUT TIMEOUT-MSECS)
> 
> Allow any pending output from subprocesses to be read by Emacs.
> It is read into the process' buffers or given to their filter functions.
> Non-nil arg PROCESS means do not return until some output has been received
> from PROCESS.
> Non-nil second arg TIMEOUT and third arg TIMEOUT-MSECS are number of
> seconds and microseconds to wait; return after that much time whether
> or not there is input.
> Return non-nil iff we received any output before the timeout expired.

I guess it would make some sense to change it to nanoseconds with
todays faster computers with high-resolution timers.  But since Emacs
has floating point support, and the "seconds" argument now can be a
float, I don't think that is necessary.

To decrease the resolution seems wrong.  This change should be
reverted, and the reference manual updated instead.

Please fix this in Emacs 22 if possible, since this change breaks
existing code.  If, for some reason, my patch is unacceptable, please
mention this as an incompatible change in the NEWS file.

    /ceder
-- 
Per Cederqvist <ceder@lysator.liu.se>

[-- Attachment #2: revert third argument of accept-process-output --]
[-- Type: text/x-patch, Size: 2392 bytes --]

Index: src/ChangeLog
===================================================================
RCS file: /sources/emacs/emacs/src/ChangeLog,v
retrieving revision 1.5658
diff -u -r1.5658 ChangeLog
--- src/ChangeLog	1 May 2007 08:15:56 -0000	1.5658
+++ src/ChangeLog	3 May 2007 12:40:24 -0000
@@ -1,3 +1,10 @@
+2007-05-03  Per Cederqvist  <ceder@lysator.liu.se>
+
+	* process.c (Faccept_process_output): Revert 2006-03-22 change so
+	that the third argument once again is in microseconds (not
+	milliseconds).  This makes it compatible with Emacs 21 and
+	earlier.  Problem found by Henrik Rindlöw.
+
 2007-05-01  YAMAMOTO Mitsuharu  <mituharu@math.s.chiba-u.ac.jp>
 
 	* macmenu.c (mac_dialog_show): Apply 2007-04-27 change for xmenu.c.
Index: src/process.c
===================================================================
RCS file: /sources/emacs/emacs/src/process.c,v
retrieving revision 1.512
diff -u -r1.512 process.c
--- src/process.c	23 Apr 2007 21:27:40 -0000	1.512
+++ src/process.c	3 May 2007 12:40:25 -0000
@@ -3912,8 +3912,8 @@
 Non-nil arg PROCESS means do not return until some output has been received
 from PROCESS.
 
-Non-nil second arg SECONDS and third arg MILLISEC are number of
-seconds and milliseconds to wait; return after that much time whether
+Non-nil second arg SECONDS and third arg MICROSEC are number of
+seconds and microseconds to wait; return after that much time whether
 or not there is input.  If SECONDS is a floating point number,
 it specifies a fractional number of seconds to wait.
 
@@ -3921,8 +3921,8 @@
 from PROCESS, suspending reading output from other processes.
 If JUST-THIS-ONE is an integer, don't run any timers either.
 Return non-nil iff we received any output before the timeout expired.  */)
-     (process, seconds, millisec, just_this_one)
-     register Lisp_Object process, seconds, millisec, just_this_one;
+     (process, seconds, microsec, just_this_one)
+     register Lisp_Object process, seconds, microsec, just_this_one;
 {
   int secs, usecs = 0;
 
@@ -3944,10 +3944,10 @@
       else
 	wrong_type_argument (Qnumberp, seconds);
 
-      if (INTEGERP (millisec))
+      if (INTEGERP (microsec))
 	{
 	  int carry;
-	  usecs += XINT (millisec) * 1000;
+	  usecs += XINT (microsec);
 	  carry = usecs / 1000000;
 	  secs += carry;
 	  if ((usecs -= carry * 1000000) < 0)

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

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

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

* Re: accept-process-output incompatible change in CVS Emacs: patch included
  2007-05-04  8:56 accept-process-output incompatible change in CVS Emacs: patch included Per Cederqvist
@ 2007-05-04 22:33 ` Kim F. Storm
  2007-05-05  8:42   ` Per Cederqvist
  0 siblings, 1 reply; 6+ messages in thread
From: Kim F. Storm @ 2007-05-04 22:33 UTC (permalink / raw)
  To: Per Cederqvist; +Cc: bug-gnu-emacs

Per Cederqvist <ceder@ingate.com> writes:

> This change in the upcoming Emacs 22 breaks some code:
>
>> 2006-03-22  Kim F. Storm  <storm@cua.dk>
>> 
>> 	* process.c (Faccept_process_output): Fix to comply with lisp
>> 	reference.  Change arg "timeout" to "seconds" and allow both
>> 	integer and float value.  Change arg "timeout-msec" to "millisec"
>> 	and interpret" as milliseconds rather than microseconds.  Fix doc
>> 	string accordingly.

Please see:

http://lists.gnu.org/archive/html/emacs-devel/2006-03/msg00937.html

>
> I enclose a patch that makes accept-process-output
> backwards-compatible with Emacs 21.
>
> One application I'm running now runs very slowly, since its carefully
> tuned timeoutes are multiplied by 1000.

Well, other code believing the lisp ref, and was broken by the old
behaviour.

So just reverting the change is no good at this time.

What application is that?  

>  If, for some reason, my patch is unacceptable, please
> mention this as an incompatible change in the NEWS file.

This is a good point.

-- 
Kim F. Storm  http://www.cua.dk

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

* Re: accept-process-output incompatible change in CVS Emacs: patch included
  2007-05-04 22:33 ` Kim F. Storm
@ 2007-05-05  8:42   ` Per Cederqvist
  2007-05-05  8:57     ` Eli Zaretskii
  2007-05-05 23:19     ` Richard Stallman
  0 siblings, 2 replies; 6+ messages in thread
From: Per Cederqvist @ 2007-05-05  8:42 UTC (permalink / raw)
  To: bug-gnu-emacs

On 5/5/07, Kim F. Storm <no-spam@cua.dk> wrote:
> Per Cederqvist <ceder@ingate.com> writes:
>
> > This change in the upcoming Emacs 22 breaks some code:
> >
> >> 2006-03-22  Kim F. Storm  <storm@cua.dk>
> >>
> >>      * process.c (Faccept_process_output): Fix to comply with lisp
> >>      reference.  Change arg "timeout" to "seconds" and allow both
> >>      integer and float value.  Change arg "timeout-msec" to "millisec"
> >>      and interpret" as milliseconds rather than microseconds.  Fix doc
> >>      string accordingly.
>
> Please see:
>
> http://lists.gnu.org/archive/html/emacs-devel/2006-03/msg00937.html

rms has already installed my patch, so I guess Gnus will have to be
fixed.  I was not aware that XEmacs used milliseconds.  Maybe the
documentation should be updated to tell everybody to use floating
point instead, as the interpretation of the msec argument varies.

> > I enclose a patch that makes accept-process-output
> > backwards-compatible with Emacs 21.
> >
> > One application I'm running now runs very slowly, since its carefully
> > tuned timeoutes are multiplied by 1000.
>
> Well, other code believing the lisp ref, and was broken by the old
> behaviour.
>
> So just reverting the change is no good at this time.

If the millisecond interpretation is once again installed, please make
the code raise an overflow error if a value larger than 1000 is
supplied.  This way, code that relies on the old behaviour will not
silently be slow, but instead raise an error so that the problem is
found and can be fixed.

> What application is that?

A client to the LysKOM system, called lyskom-elisp-client.  Info about
the entire system is available from
http://www.lysator.liu.se/lyskom/index-en.html, and the client itself
can be found in ftp://ftp.lysator.liu.se/pub/lyskom/elisp-client/.

> >  If, for some reason, my patch is unacceptable, please
> > mention this as an incompatible change in the NEWS file.
>
> This is a good point.
>
> --
> Kim F. Storm  http://www.cua.dk

    /ceder

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

* Re: accept-process-output incompatible change in CVS Emacs: patch included
  2007-05-05  8:42   ` Per Cederqvist
@ 2007-05-05  8:57     ` Eli Zaretskii
  2007-05-05 23:19       ` Richard Stallman
  2007-05-05 23:19     ` Richard Stallman
  1 sibling, 1 reply; 6+ messages in thread
From: Eli Zaretskii @ 2007-05-05  8:57 UTC (permalink / raw)
  To: Per Cederqvist; +Cc: bug-gnu-emacs

> Date: Sat, 5 May 2007 10:42:43 +0200
> From: "Per Cederqvist" <ceder@ingate.com>
> 
> > What application is that?
> 
> A client to the LysKOM system, called lyskom-elisp-client.  Info about
> the entire system is available from
> http://www.lysator.liu.se/lyskom/index-en.html, and the client itself
> can be found in ftp://ftp.lysator.liu.se/pub/lyskom/elisp-client/.

So we've broken Gnus for the benefit of an obscure unbundled package.
Hardly a good decision, I'd say.

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

* Re: accept-process-output incompatible change in CVS Emacs: patch included
  2007-05-05  8:42   ` Per Cederqvist
  2007-05-05  8:57     ` Eli Zaretskii
@ 2007-05-05 23:19     ` Richard Stallman
  1 sibling, 0 replies; 6+ messages in thread
From: Richard Stallman @ 2007-05-05 23:19 UTC (permalink / raw)
  To: Per Cederqvist; +Cc: bug-gnu-emacs

    > http://lists.gnu.org/archive/html/emacs-devel/2006-03/msg00937.html

    rms has already installed my patch, so I guess Gnus will have to be
    fixed.  I was not aware that XEmacs used milliseconds.

I didn't know that Gnus depended on the newer behavior
(or about XEmacs).

Both options are quite undesirable.  Damn.  But I think it is less bad
to go with the milliseconds.  So I will take out your patch.

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

* Re: accept-process-output incompatible change in CVS Emacs: patch included
  2007-05-05  8:57     ` Eli Zaretskii
@ 2007-05-05 23:19       ` Richard Stallman
  0 siblings, 0 replies; 6+ messages in thread
From: Richard Stallman @ 2007-05-05 23:19 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: ceder, bug-gnu-emacs

    So we've broken Gnus for the benefit of an obscure unbundled package.
    Hardly a good decision, I'd say.

Give me a break!  I didn't know Gnus depended on it.

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

end of thread, other threads:[~2007-05-05 23:19 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-05-04  8:56 accept-process-output incompatible change in CVS Emacs: patch included Per Cederqvist
2007-05-04 22:33 ` Kim F. Storm
2007-05-05  8:42   ` Per Cederqvist
2007-05-05  8:57     ` Eli Zaretskii
2007-05-05 23:19       ` Richard Stallman
2007-05-05 23:19     ` Richard Stallman

Code repositories for project(s) associated with this external index

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

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.