carriage-return no longer works quite right in shell-mode

carriage-return no longer works quite right in shell-mode

[Chris Moore]

I recently updated Emacs from the CVS trunk and one of the programs I
run inside a shell-mode buffer has stopped formatting its output
correctly.

It uses carriage return characters to re-write the same line over and
over, and leaves the cursor at the beginning of the line.  Here's a
simplified example that no longer works in Emacs:

#include <stdio.h>
main() {
  int i;
  for (i = 0; i < 10; i++) {
    printf("  count: %d\r", i);
    fflush(stdout);
    sleep(1);
  }
  printf("\n");
}

Instead of re-writing the same part of the buffer over and over, I see:
    count: 0  count: 1  count: 2
in the buffer - the carriage return isn't returning the carriage like
it used to.

I notice that apt-get does still work - that leaves the cursor at the
end of the line, which I guess is the important difference.

Re: carriage-return no longer works quite right in shell-mode

[Richard Stallman]

Would someone please investigate this, DTRT, then ack?  I'd guess it
was broken by unicode-2 merge.

Message-ID: <a9691ee20802051103s575665bh105b77b6f6088443@mail.gmail.com>
Date: Tue, 5 Feb 2008 20:03:58 +0100
From: "Chris Moore" <christopher.ian.moore@gmail.com>
To: emacs-pretest-bug@gnu.org
MIME-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1
Content-Disposition: inline
Cc: 
Subject: carriage-return no longer works quite right in shell-mode

I recently updated Emacs from the CVS trunk and one of the programs I
run inside a shell-mode buffer has stopped formatting its output
correctly.

It uses carriage return characters to re-write the same line over and
over, and leaves the cursor at the beginning of the line.  Here's a
simplified example that no longer works in Emacs:

#include <stdio.h>
main() {
  int i;
  for (i = 0; i < 10; i++) {
    printf("  count: %d\r", i);
    fflush(stdout);
    sleep(1);
  }
  printf("\n");
}

Instead of re-writing the same part of the buffer over and over, I see:
    count: 0  count: 1  count: 2
in the buffer - the carriage return isn't returning the carriage like
it used to.

I notice that apt-get does still work - that leaves the cursor at the
end of the line, which I guess is the important difference.

Re: carriage-return no longer works quite right in shell-mode

[Stefan Monnier]

> I recently updated Emacs from the CVS trunk and one of the programs
> I run inside a shell-mode buffer has stopped formatting its
> output correctly.

How old was your previous Emacs?

> It uses carriage return characters to re-write the same line over and
> over, and leaves the cursor at the beginning of the line.  Here's a
> simplified example that no longer works in Emacs:

> #include <stdio.h>
> main() {
>   int i;
>   for (i = 0; i < 10; i++) {
>     printf("  count: %d\r", i);
>     fflush(stdout);
>     sleep(1);
>   }
>   printf("\n");
> }

> Instead of re-writing the same part of the buffer over and over, I see:
>     count: 0  count: 1  count: 2
> in the buffer - the carriage return isn't returning the carriage like
> it used to.

Hmm... I see that indeed.

> I notice that apt-get does still work - that leaves the cursor at the
> end of the line, which I guess is the important difference.

Yes.  Funnily enough, in Emacs-22 the two behave identically because
when your program sends "count: N\r", Emacs passes the "count: N" to the
`shell' package, but keeps the \r for later because it needs to first
see if the following char is a \n or not in order to know whether to
drop the \r or not.  I.e. the details of decoding DOS-style EOLs caused
Emacs to turn one form into the other.

So it indeed, looks like a problem introduced by the unicode merge: for
some reason, the unicode code just sends the \r eagerly before it can
tell whether the next char is a \n.
I'll keep it in my todo list, but it may take me a little while to get
back to it, so if someone wants to do it before, he's welcome,


        Stefan

Re: carriage-return no longer works quite right in shell-mode

[Chris Moore]

On Feb 11, 2008 3:55 AM, Stefan Monnier <monnier@iro.umontreal.ca> wrote:
> > I recently updated Emacs from the CVS trunk and one of the programs
> > I run inside a shell-mode buffer has stopped formatting its
> > output correctly.
>
> How old was your previous Emacs?

I update from the CVS trunk about once per week, so it was around a week old.

Re: carriage-return no longer works quite right in shell-mode

[Romain Francoise]

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

> So it indeed, looks like a problem introduced by the unicode merge: for
> some reason, the unicode code just sends the \r eagerly before it can
> tell whether the next char is a \n.
> I'll keep it in my todo list, but it may take me a little while to get
> back to it, so if someone wants to do it before, he's welcome,

This bug is very annoying...

Handa-san, perhaps you have an idea about what broke this, and where
someone could start investigating?

Thanks.

Re: carriage-return no longer works quite right in shell-mode

[Kenichi Handa]

In article <87wsoow3zx.fsf@elegiac.orebokech.com>, Romain Francoise <romain@orebokech.com> writes:

> Stefan Monnier <monnier@iro.umontreal.ca> writes:
> > So it indeed, looks like a problem introduced by the unicode merge: for
> > some reason, the unicode code just sends the \r eagerly before it can
> > tell whether the next char is a \n.
> > I'll keep it in my todo list, but it may take me a little while to get
> > back to it, so if someone wants to do it before, he's welcome,

> This bug is very annoying...

> Handa-san, perhaps you have an idea about what broke this, and where
> someone could start investigating?

Sorry for not responding on this thread.

I found two problems are related.

One is that the default-process-coding-system is now set to
XXX-unix, but previously it doesn't specify eol-format
(i.e. auto-detect).  comint.el changes the eol-format to
XXX-dos only if eol-format is not specifed.

The other is in the new code conversion routine as Stefan
wrote above.

I'll fix the latter bug in haste.  Could someone figure out
why default-process-coding-system specifies eol-format now?

By the way, I think the way of processing CR in comint is
not good.  For instance, with the follwoing program, you
can't see the tailing "times" in *shell*.

#include <stdio.h>
main() {
  int i;
  printf ("  times");
  for (i = 0; i < 10; i++) {
    printf("\r%d", i);
    fflush(stdout);
    sleep(1);
  }
  printf("\n");
}

I think it should be modified not to rely on the fact that
the decoding of CR is suspended until the next byte arrives.

---
Kenichi Handa
handa@ni.aist.go.jp

Re: carriage-return no longer works quite right in shell-mode

[Romain Francoise]

Kenichi Handa <handa@m17n.org> writes:

> Could someone figure out why default-process-coding-system
> specifies eol-format now?

Could it be a side effect of this change?  (The ChangeLog entry is
from the trunk, the change itself is from the Unicode branch.)

2008-02-01  Kenichi Handa  <handa@m17n.org>

	* coding.c (coding_inherit_eol_type): If PARENT is nil, inherit from
	system_eol_type.
	(syms_of_coding): Initialize system_eol_type.

	* process.c (Fset_process_coding_system): Inherit system's eol
	format if necessary.

Re: carriage-return no longer works quite right in shell-mode

[Kenichi Handa]

In article <87hcfrwzyw.fsf@elegiac.orebokech.com>, Romain Francoise <romain@orebokech.com> writes:

> Kenichi Handa <handa@m17n.org> writes:
> > Could someone figure out why default-process-coding-system
> > specifies eol-format now?

> Could it be a side effect of this change?  (The ChangeLog entry is
> from the trunk, the change itself is from the Unicode branch.)

> 2008-02-01  Kenichi Handa  <handa@m17n.org>

> 	* coding.c (coding_inherit_eol_type): If PARENT is nil, inherit from
> 	system_eol_type.
> 	(syms_of_coding): Initialize system_eol_type.

> 	* process.c (Fset_process_coding_system): Inherit system's eol
> 	format if necessary.

But, Fset_process_coding_system makes only the coding system
for ENCODING inheirt the system EOL type.

---
Kenichi Handa
handa@ni.aist.go.jp

Re: carriage-return no longer works quite right in shell-mode

[Kenichi Handa]

In article <E1JV3Su-0008Ul-0O@etlken.m17n.org>, Kenichi Handa <handa@m17n.org> writes:

> I found two problems are related.

> One is that the default-process-coding-system is now set to
> XXX-unix, but previously it doesn't specify eol-format
> (i.e. auto-detect).  comint.el changes the eol-format to
> XXX-dos only if eol-format is not specifed.

> The other is in the new code conversion routine as Stefan
> wrote above.

> I'll fix the latter bug in haste.  Could someone figure out
> why default-process-coding-system specifies eol-format now?

I installed a fix for that.  You can verify that by starting
the shell mode, and explicitly set the process coding system
for decoding to XXX-dos.

> By the way, I think the way of processing CR in comint is
> not good.  For instance, with the follwoing program, you
> can't see the tailing "times" in *shell*.

> #include <stdio.h>
> main() {
>   int i;
>   printf ("  times");
>   for (i = 0; i < 10; i++) {
>     printf("\r%d", i);
>     fflush(stdout);
>     sleep(1);
>   }
>   printf("\n");
> }

> I think it should be modified not to rely on the fact that
> the decoding of CR is suspended until the next byte arrives.

I propose the following change to comint.el.  Although I
think the new code improve the handling of CR, I have not
yet committed it because comint-output-filter (the caller of
comint-carriage-motion) does complicated marker and overlay
adjustment, and I'm not that confident that the new code
doesn't make the adjustment breaks.  Could someone who is
familiar with comint-output-filter check it?

2008-03-04 Kenichi Handa  <handa@ni.aist.go.jp>

	* comint.el (comint-exec-1): Don't change the coding-system for
	decoding to dos-like EOL.
	(comint-carriage-motion): Fully rewrite.

Index: comint.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/comint.el,v
retrieving revision 1.373
diff -c -r1.373 comint.el
*** comint.el	22 Jan 2008 23:53:43 -0000	1.373
--- comint.el	4 Mar 2008 11:41:44 -0000
***************
*** 800,811 ****
      (let ((coding-systems (process-coding-system proc)))
        (setq decoding (car coding-systems)
  	    encoding (cdr coding-systems)))
-     ;; If start-file-process decided to use some coding system for decoding
-     ;; data sent from the process and the coding system doesn't
-     ;; specify EOL conversion, we had better convert CRLF to LF.
-     (if (vectorp (coding-system-eol-type decoding))
- 	(setq decoding (coding-system-change-eol-conversion decoding 'dos)
- 	      changed t))
      ;; Even if start-file-process left the coding system for encoding data
      ;; sent from the process undecided, we had better use the same one
      ;; as what we use for decoding.  But, we should suppress EOL
--- 800,805 ----
***************
*** 1674,1706 ****
  Make single carriage returns delete to the beginning of the line.
  Make backspaces delete the previous character."
    (save-excursion
!     ;; First do a quick check to see if there are any applicable
!     ;; characters, so we can avoid calling save-match-data and
!     ;; save-restriction if not.
      (goto-char start)
!     (when (< (skip-chars-forward "^\b\r" end) (- end start))
!       (save-match-data
! 	(save-restriction
! 	  (widen)
! 	  (let ((inhibit-field-text-motion t)
! 		(inhibit-read-only t))
! 	    ;; CR LF -> LF
! 	    ;; Note that this won't work properly when the CR and LF
! 	    ;; are in different output chunks, but this is probably an
! 	    ;; exceedingly rare case (because they are generally
! 	    ;; written as a unit), and to delay interpretation of a
! 	    ;; trailing CR in a chunk would result in odd interactive
! 	    ;; behavior (and this case is probably far more common).
! 	    (while (re-search-forward "\r$" end t)
! 	      (delete-char -1))
! 	    ;; bare CR -> delete preceding line
! 	    (goto-char start)
! 	    (while (search-forward "\r" end t)
! 	      (delete-region (point) (line-beginning-position)))
! 	    ;; BS -> delete preceding character
! 	    (goto-char start)
! 	    (while (search-forward "\b" end t)
! 	      (delete-char -2))))))))
  
  ;; The purpose of using this filter for comint processes
  ;; is to keep comint-last-input-end from moving forward
--- 1668,1723 ----
  Make single carriage returns delete to the beginning of the line.
  Make backspaces delete the previous character."
    (save-excursion
!     ;; We used to check the existence of \b and \r at first to avoid
!     ;; calling save-match-data and save-restriction.  But, such a
!     ;; check is not necessary now because we don't use regexp search
!     ;; nor save-restriction.  Note that the buffer is already widen,
!     ;; and calling narrow-to-region and widen are not that heavy.
      (goto-char start)
!     (let* ((inhibit-field-text-motion t)
! 	   (inhibit-read-only t)
! 	   (lbeg (line-beginning-position))
! 	   delete-end ch)
!       ;; If the preceding text is marked as "must-overwrite", record
!       ;; it in delete-end.
!       (when (and (> start (point-min))
! 		 (get-text-property (1- start) 'comint-must-overwrite))
! 	(setq delete-end (point-marker))
! 	(remove-text-properties (1- start) start '(comint-must-overwrite nil)))
!       (narrow-to-region lbeg end)
!       ;; Handle BS, LF, and CR specially.
!       (while (and (skip-chars-forward "^\b\n\r") (not (eobp)))
! 	(setq ch (following-char))
! 	(cond ((= ch ?\b)		; CH = BS
! 	       (delete-char 1)
! 	       (if (> (point) lbeg)
! 		   (delete-char -1)))
! 	      ((= ch ?\n)
! 	       (when delete-end		; CH = LF
! 		 (if (< delete-end (point))
! 		     (delete-region lbeg delete-end))
! 		 (set-marker delete-end nil)
! 		 (setq delete-end nil))
! 	       (forward-char 1)
! 	       (setq lbeg (point)))
! 	      (t			; CH = CR
! 	       (delete-char 1)
! 	       (if delete-end
! 		   (when (< delete-end (point))
! 		     (delete-region lbeg delete-end)
! 		     (move-marker delete-end (point)))
! 		 (setq delete-end (point-marker))))))
!       (when delete-end
! 	(if (< delete-end (point))
! 	    ;; As there's a text after the last CR, make the current
! 	    ;; line contain only that text.
! 	    (delete-region lbeg delete-end)
! 	  ;; Remember that the process output ends by CR, and thus we
! 	  ;; must overwrite the contents of the current line next
! 	  ;; time.
! 	  (put-text-property lbeg delete-end 'comint-must-overwrite t))
! 	(set-marker delete-end nil))
!       (widen))))
  
  ;; The purpose of using this filter for comint processes
  ;; is to keep comint-last-input-end from moving forward

Re: carriage-return no longer works quite right in shell-mode

[Stefan Monnier]

> I propose the following change to comint.el.  Although I
> think the new code improve the handling of CR, I have not
> yet committed it because comint-output-filter (the caller of
> comint-carriage-motion) does complicated marker and overlay
> adjustment, and I'm not that confident that the new code
> doesn't make the adjustment breaks.  Could someone who is
> familiar with comint-output-filter check it?

Thanks for looking into it.
I think you may as well commit it.


        Stefan

Re: carriage-return no longer works quite right in shell-mode

[Kenichi Handa]

In article <jwvve42biia.fsf-monnier+emacs@gnu.org>, Stefan Monnier <monnier@iro.umontreal.ca> writes:

> > I propose the following change to comint.el.  Although I
> > think the new code improve the handling of CR, I have not
> > yet committed it because comint-output-filter (the caller of
> > comint-carriage-motion) does complicated marker and overlay
> > adjustment, and I'm not that confident that the new code
> > doesn't make the adjustment breaks.  Could someone who is
> > familiar with comint-output-filter check it?

> Thanks for looking into it.
> I think you may as well commit it.

Ok, done.

---
Kenichi Handa
handa@ni.aist.go.jp