bug#1973: Bug in simple.el (Emacs version 22.2.1)

bug#1973: Bug in simple.el (Emacs version 22.2.1)

[Sebastian Tennant]

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

Hello all,

Asynchronous commands called via shell-command, for example:

 (shell-command "apt-get update &")

fill the buffer *Async Shell Command Output* with Ctrl-Ms, which I'm
sure is not what's intended.

My patch (attached) fixes this by using make-comint-in-buffer, rather
than start-process, to call the asynchronous process.

HTH,

Sebastian


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

--- /usr/share/emacs/22.2/lisp/simple-patched.el	2009-01-20 19:51:29.528333079 +0000
+++ /usr/share/emacs/22.2/lisp/simple.el	2008-03-11 01:57:32.000000000 +0000
@@ -1892,11 +1892,10 @@
 		  (erase-buffer)
 		  (display-buffer buffer)
 		  (setq default-directory directory)
-		  (setq proc (get-buffer-process
-			      (make-comint-in-buffer
-			       "Shell" buffer shell-file-name nil
-			       shell-command-switch command)))
+		  (setq proc (start-process "Shell" buffer shell-file-name
+					    shell-command-switch command))
 		  (setq mode-line-process '(":%s"))
+		  (require 'shell) (shell-mode)
 		  (set-process-sentinel proc 'shell-command-sentinel)
 		  ))
 	    (shell-command-on-region (point) (point) command

bug#1973: Bug in simple.el (Emacs version 22.2.1)

[Kevin Rodgers]

Sebastian Tennant wrote:
> Hello all,
> 
> Asynchronous commands called via shell-command, for example:
> 
>  (shell-command "apt-get update &")
> 
> fill the buffer *Async Shell Command Output* with Ctrl-Ms, which I'm
> sure is not what's intended.
> 
> My patch (attached) fixes this by using make-comint-in-buffer, rather
> than start-process, to call the asynchronous process.

I don't experience that with emacs -Q on:

GNU Emacs 22.3.1 (i386-apple-darwin8.11.1, Carbon Version 1.6.0)

What does `C-h C RET' in the *Async Shell Command Output* show?  Mine
says:

Coding system for saving this buffer:
   Not set locally, use the default.
Default coding system (for new files):
   1 -- iso-latin-1 (alias: iso-8859-1 latin-1)

Coding system for keyboard input:
   nil
Coding system for terminal output:
   1 -- iso-8859-1 (alias of iso-latin-1)

Defaults for subprocess I/O:
   decoding: 1 -- iso-latin-1 (alias: iso-8859-1 latin-1)

   encoding: 1 -- iso-latin-1 (alias: iso-8859-1 latin-1)


Priority order for recognizing coding systems when reading files:
   1. iso-latin-1 (alias: iso-8859-1 latin-1)
   2. mule-utf-8 (alias: utf-8)
   3. mule-utf-16be-with-signature (alias: utf-16be-with-signature 
mule-utf-16-be utf-16-be)
   4. mule-utf-16le-with-signature (alias: utf-16le-with-signature 
mule-utf-16-le utf-16-le)
   5. iso-2022-jp (alias: junet)
   6. iso-2022-7bit
   7. iso-2022-7bit-lock (alias: iso-2022-int-1)
   8. iso-2022-8bit-ss2
   9. emacs-mule
   10. raw-text
   11. japanese-shift-jis (alias: shift_jis sjis cp932)
   12. chinese-big5 (alias: big5 cn-big5 cp950)
   13. no-conversion

   Other coding systems cannot be distinguished automatically
   from these, and therefore cannot be recognized automatically
   with the present coding system priorities.

   The following are decoded correctly but recognized as iso-2022-7bit-lock:
     iso-2022-7bit-ss2 iso-2022-7bit-lock-ss2 iso-2022-cn iso-2022-cn-ext
     iso-2022-jp-2 iso-2022-kr

Particular coding systems specified for certain file names:

   OPERATION	TARGET PATTERN		CODING SYSTEM(s)
   ---------	--------------		----------------
   File I/O	"\\.dz\\'"		(no-conversion . no-conversion)
		"\\.g?z\\(~\\|\\.~[0-9]+~\\)?\\'"
					(no-conversion . no-conversion)
		"\\.tgz\\'"		(no-conversion . no-conversion)
		"\\.tbz\\'"		(no-conversion . no-conversion)
		"\\.bz2\\(~\\|\\.~[0-9]+~\\)?\\'"
					(no-conversion . no-conversion)
		"\\.Z\\(~\\|\\.~[0-9]+~\\)?\\'"
					(no-conversion . no-conversion)
		"\\.elc\\'"		(emacs-mule . emacs-mule)
		"\\.utf\\(-8\\)?\\'"	utf-8
		"\\(\\`\\|/\\)loaddefs.el\\'"
					(raw-text . raw-text-unix)
		"\\.tar\\'"		(no-conversion . no-conversion)
		"\\.po[tx]?\\'\\|\\.po\\."
					po-find-file-coding-system
		"\\.\\(tex\\|ltx\\|dtx\\|drv\\)\\'"
					latexenc-find-file-coding-system
		""			(undecided)
   Process I/O	nothing specified
   Network I/O	nothing specified

-- 
Kevin Rodgers
Denver, Colorado, USA

bug#1973: Bug in simple.el (Emacs version 22.2.1)

[Sebastian Tennant]

Quoth Kevin Rodgers <kevin.d.rodgers@gmail.com>:
> What does `C-h C RET' in the *Async Shell Command Output* show?  

Coding system for saving this buffer:
  Not set locally, use the default.
Default coding system (for new files):
  u -- mule-utf-8 (alias: utf-8)

Coding system for keyboard input:
  u -- utf-8 (alias of mule-utf-8)

Coding system for terminal output:
  u -- utf-8 (alias of mule-utf-8)

Defaults for subprocess I/O:
  decoding: u -- mule-utf-8 (alias: utf-8)

  encoding: u -- mule-utf-8 (alias: utf-8)


Priority order for recognizing coding systems when reading files:
  1. mule-utf-8 (alias: utf-8)
  2. iso-latin-1 (alias: iso-8859-1 latin-1)
  3. mule-utf-16be-with-signature (alias: utf-16be-with-signature mule-utf-16-be utf-16-be)
  4. mule-utf-16le-with-signature (alias: utf-16le-with-signature mule-utf-16-le utf-16-le)
  5. iso-2022-jp (alias: junet)
  6. iso-2022-7bit 
  7. iso-2022-7bit-lock (alias: iso-2022-int-1)
  8. iso-2022-8bit-ss2 
  9. emacs-mule 
  10. raw-text 
  11. japanese-shift-jis (alias: shift_jis sjis cp932)
  12. chinese-big5 (alias: big5 cn-big5 cp950)
  13. no-conversion 

  Other coding systems cannot be distinguished automatically
  from these, and therefore cannot be recognized automatically
  with the present coding system priorities.

  The following are decoded correctly but recognized as iso-2022-7bit-lock:
    iso-2022-7bit-ss2 iso-2022-7bit-lock-ss2 iso-2022-cn iso-2022-cn-ext iso-2022-jp-2 iso-2022-kr

Particular coding systems specified for certain file names:

  OPERATION	TARGET PATTERN		CODING SYSTEM(s)
  ---------	--------------		----------------
  File I/O	"\\.dz\\'"		(no-conversion . no-conversion)
		"\\.g?z\\(~\\|\\.~[0-9]+~\\)?\\'"
					(no-conversion . no-conversion)
		"\\.tgz\\'"		(no-conversion . no-conversion)
		"\\.tbz\\'"		(no-conversion . no-conversion)
		"\\.bz2\\(~\\|\\.~[0-9]+~\\)?\\'"
					(no-conversion . no-conversion)
		"\\.Z\\(~\\|\\.~[0-9]+~\\)?\\'"
					(no-conversion . no-conversion)
		"\\.elc\\'"		(emacs-mule . emacs-mule)
		"\\.utf\\(-8\\)?\\'"	utf-8
		"\\(\\`\\|/\\)loaddefs.el\\'"
					(raw-text . raw-text-unix)
		"\\.tar\\'"		(no-conversion . no-conversion)
		"\\.po[tx]?\\'\\|\\.po\\."
					po-find-file-coding-system
		"\\.\\(tex\\|ltx\\|dtx\\|drv\\)\\'"
					latexenc-find-file-coding-system
		""			(undecided)
  Process I/O	nothing specified
  Network I/O	nothing specified


My ~/.profile contains the line 'export LANG=en_GB.UTF-8' which may
explain some of the differences, but it still ain't right.

Sebastian

bug#1973: Bug in simple.el (Emacs version 22.2.1)

[Stefan Monnier]

> Asynchronous commands called via shell-command, for example:

>  (shell-command "apt-get update &")

> fill the buffer *Async Shell Command Output* with Ctrl-Ms, which I'm
> sure is not what's intended.

> My patch (attached) fixes this by using make-comint-in-buffer, rather
> than start-process, to call the asynchronous process.

Such a feature would be good, indeed.  I'm not sure that just using
make-comint-in-buffer is good enough.  Maybe we should provide a new
command instead, along the lines of comint-run, or maybe even change
comint-run so that it accepts a shell command rather than an
executable name.


        Stefan

bug#1973: Bug in simple.el (Emacs version 22.2.1)

[Sebastian Tennant]

Quoth Stefan Monnier <monnier@iro.umontreal.ca>:
>> Asynchronous commands called via shell-command, for example:
>
>>  (shell-command "apt-get update &")
>
>> fill the buffer *Async Shell Command Output* with Ctrl-Ms, which I'm
>> sure is not what's intended.
>
>> My patch (attached) fixes this by using make-comint-in-buffer, rather
>> than start-process, to call the asynchronous process.
>
> Such a feature would be good, indeed.  

Sorry Stefan, I'm not sure what feature it is you are refering to.

There seems to be a bug within shell mode that prevents it from handling
Ctrl-Ms properly.

> I'm not sure that just using make-comint-in-buffer is good enough.
> Maybe we should provide a new command instead, along the lines of
> comint-run, or maybe even change comint-run so that it accepts a shell
> command rather than an executable name.

Why?

In any case, I suspect we should extend rather than alter, comint mode
being the basis for dozens of other 'interpreter modes'.

Sebastian

bug#1973: Bug in simple.el (Emacs version 22.2.1)

[Sebastian Tennant]

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

Quoth Sebastian Tennant <sebyte@smolny.plus.com>:
> Asynchronous commands called via shell-command, for example:
>
>  (shell-command "apt-get update &")
>
> fill the buffer *Async Shell Command Output* with Ctrl-Ms, which I'm
> sure is not what's intended.
>
> My patch (attached) fixes this by using make-comint-in-buffer, rather
> than start-process, to call the asynchronous process.

Actually, this updated patch (attached) is what's required to fix the
problem here.

It explicitly puts the buffer in comint-mode.  (My last patch neglected
to do this).

Sebastian


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

--- /usr/share/emacs/22.2/lisp/simple.el~	2009-01-21 15:12:12.502915265 +0000
+++ /usr/share/emacs/22.2/lisp/simple.el	2009-01-22 13:22:17.352984134 +0000
@@ -1889,13 +1889,14 @@
 		      (error "Shell command in progress")))
 		(with-current-buffer buffer
 		  (setq buffer-read-only nil)
-		  (erase-buffer)
+		  (comint-mode) (erase-buffer)
 		  (display-buffer buffer)
 		  (setq default-directory directory)
-		  (setq proc (start-process "Shell" buffer shell-file-name
-					    shell-command-switch command))
+		  (setq proc (get-buffer-process
+			      (make-comint-in-buffer
+			       "Shell" buffer shell-file-name nil
+			       shell-command-switch command)))
 		  (setq mode-line-process '(":%s"))
-		  (require 'shell) (shell-mode)
 		  (set-process-sentinel proc 'shell-command-sentinel)
 		  ))
 	    (shell-command-on-region (point) (point) command

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


-- 
Emacs' AlsaPlayer - Music Without Jolts
Lightweight, full-featured and mindful of your idyllic happiness.
http://home.gna.org/eap

bug#1973: Bug in simple.el (Emacs version 22.2.1)

[Stefan Monnier]

>>> Asynchronous commands called via shell-command, for example:
>>> (shell-command "apt-get update &")
>>> fill the buffer *Async Shell Command Output* with Ctrl-Ms, which I'm
>>> sure is not what's intended.
>> 
>>> My patch (attached) fixes this by using make-comint-in-buffer, rather
>>> than start-process, to call the asynchronous process.
>> 
>> Such a feature would be good, indeed.  

> Sorry Stefan, I'm not sure what feature it is you are refering to.

The feature of interpreting ^M in the output.

> There seems to be a bug within shell mode that prevents it from handling
> Ctrl-Ms properly.

It's not a bug.  shell-command just runs a command and gives you the output
of the command.  Some commands presume the output is displayed by some
kind of terminal and will use terminal commands like ^M (or even ANSI
color escape sequences), whereas shell-command was not designed for
this purpose.

>> I'm not sure that just using make-comint-in-buffer is good enough.
>> Maybe we should provide a new command instead, along the lines of
>> comint-run, or maybe even change comint-run so that it accepts a shell
>> command rather than an executable name.
> Why?

Because I'm not sure in what kinds of circumstances people use
shell-command with an "&" at the end.

> In any case, I suspect we should extend rather than alter, comint mode
> being the basis for dozens of other 'interpreter modes'.

`comint-run' is a command (i.e. for interactive use).  I do see that it
is occasionally misused from Elisp code (which should use make-comint
instead), so we should be somewhat careful indeed when changing it.


        Stefan

bug#1973: Bug in simple.el (Emacs version 22.2.1)

[Sebastian Tennant]

Quoth "Stefan Monnier" <monnier@iro.umontreal.ca>:
>> There seems to be a bug within shell mode that prevents it from handling
>> Ctrl-Ms properly.
>
> It's not a bug.  shell-command just runs a command and gives you the output
> of the command.  Some commands presume the output is displayed by some
> kind of terminal and will use terminal commands like ^M (or even ANSI
> color escape sequences), whereas shell-command was not designed for
> this purpose.

So the shell mode used in interactive shells (started with M-x shell) is
enhanced somehow to handle ^Ms?

Note that Kevin Rodgers does not observe the same behaviour:

 http://article.gmane.org/gmane.emacs.bugs/24348
 
>>> I'm not sure that just using make-comint-in-buffer is good enough.
>>> Maybe we should provide a new command instead, along the lines of
>>> comint-run, or maybe even change comint-run so that it accepts a shell
>>> command rather than an executable name.
>>
>> Why?
>
> Because I'm not sure in what kinds of circumstances people use
> shell-command with an "&" at the end.

I use it for commands that take a while to complete, so that I can
continue doing other things and/or watch what's going on.  I expect the
buffer to behave/look/feel as much like an ordinary shell as possible.
Perhaps this is not typical.  I don't know.

>> In any case, I suspect we should extend rather than alter, comint mode
>> being the basis for dozens of other 'interpreter modes'.
>
> `comint-run' is a command (i.e. for interactive use).  I do see that it
> is occasionally misused from Elisp code (which should use make-comint
> instead), so we should be somewhat careful indeed when changing it.

Noted (and agreed)

Sebastian

-- 
Emacs' AlsaPlayer - Music Without Jolts
Lightweight, full-featured and mindful of your idyllic happiness.
http://home.gna.org/eap

bug#1973: Bug in simple.el (Emacs version 22.2.1)

[Sebastian Tennant]

Quoth Sebastian Tennant <sebyte@smolny.plus.com>:
> Actually, this updated patch (attached) is what's required to fix the
> problem here.
>
> It explicitly puts the buffer in comint-mode.  (My last patch neglected
> to do this).

Correction.  The patch does not requre a call to comint-mode.  The
problem is stranger still.

With the first patch I provided (no comint-mode call) applied:

 $ emacs -Q

 M-! apt-get update &
 =| ^Ms abound
 
 M-x load-library RET simple.el RET
 => t
 M-! apt-get update &
 =| no ^Ms (in Shell mode)

I removed simple.elc before performing the above test so that's not the
problem.

With the unpatched simple.el in place, loading it manually a second time
does not fix the problem so my patch (with or without the call to
comint-mode) is still required.

Sebastian
-- 
Emacs' AlsaPlayer - Music Without Jolts
Lightweight, full-featured and mindful of your idyllic happiness.
http://home.gna.org/eap

bug#1973: Bug in simple.el (Emacs version 22.2.1)

[Stefan Monnier]

> So the shell mode used in interactive shells (started with M-x shell) is
> enhanced somehow to handle ^Ms?

It's actually done in comint-mode rather than just in shell-mode.
See comint-carriage-motion.  Note that it's an approximation of the
actual semantics of ^M in a terminal, because comint-mode doesn't
actually provide the usual semantics of a terminal.

> Note that Kevin Rodgers does not observe the same behaviour:
>  http://article.gmane.org/gmane.emacs.bugs/24348

I'm not sure what he's referring to.


        Stefan

bug#1973: Bug in simple.el (Emacs version 22.2.1)

[Sebastian Tennant]

Quoth "Stefan Monnier" <monnier@iro.umontreal.ca>:
>> So the shell mode used in interactive shells (started with M-x shell) is
>> enhanced somehow to handle ^Ms?
>
> It's actually done in comint-mode rather than just in shell-mode.
> See comint-carriage-motion.  Note that it's an approximation of the
> actual semantics of ^M in a terminal, because comint-mode doesn't
> actually provide the usual semantics of a terminal.

Hmm... interesting.  I hadn't noticed the comint-carriage-motion
function before.

So Shell mode (in buffer *shell* for instance) is actually Comint mode
masquerading as Shell mode whereas buffer *Async Command Output* is
plain Shell mode, not masquerading as anything?

>> Note that Kevin Rodgers does not observe the same behaviour:
>>  http://article.gmane.org/gmane.emacs.bugs/24348
>
> I'm not sure what he's referring to.

His *Async Command Output* buffer does _not_ display ^M chars when he
issues calls to shell-command with arguments ending "&".

Seb
-- 
Emacs' AlsaPlayer - Music Without Jolts
Lightweight, full-featured and mindful of your idyllic happiness.
http://home.gna.org/eap

bug#1973: Bug in simple.el (Emacs version 22.2.1)

[Stefan Monnier]

> So Shell mode (in buffer *shell* for instance) is actually Comint mode
> masquerading as Shell mode

More specifically, shell-mode is a mode derived from comint-mode
(i.e. a submode).

> whereas buffer *Async Command Output* is
> plain Shell mode, not masquerading as anything?

I guess it's using shell-mode, but somehow fails to setup the process's
output filter?

>>> Note that Kevin Rodgers does not observe the same behaviour:
>>> http://article.gmane.org/gmane.emacs.bugs/24348
>> I'm not sure what he's referring to.
> His *Async Command Output* buffer does _not_ display ^M chars when he
> issues calls to shell-command with arguments ending "&".

I don't know how you reach that conclusion.  It's a possible
interpretation, but it's definitely not the only one.


        Stefan

bug#1973: Bug in simple.el (Emacs version 22.2.1)

[Sebastian Tennant]

Quoth "Stefan Monnier" <monnier@iro.umontreal.ca>:
>> So Shell mode (in buffer *shell* for instance) is actually Comint mode
>> masquerading as Shell mode
>
> More specifically, shell-mode is a mode derived from comint-mode
> (i.e. a submode).
>
>> whereas buffer *Async Command Output* is
>> plain Shell mode, not masquerading as anything?
>
> I guess it's using shell-mode, but somehow fails to setup the process's
> output filter?

Now you're making it sound like a bug, just as I'm starting to accept
that it's a misfeature :)

>>>> Note that Kevin Rodgers does not observe the same behaviour:
>>>> http://article.gmane.org/gmane.emacs.bugs/24348
>>> I'm not sure what he's referring to.
>> His *Async Command Output* buffer does _not_ display ^M chars when he
>> issues calls to shell-command with arguments ending "&".
>
> I don't know how you reach that conclusion.  It's a possible
> interpretation, but it's definitely not the only one.

You're right, it's not clear what he means.

I started a thread on gmane.emacs.help to clear this up once and for
all:

 http://article.gmane.org/gmane.emacs.help/61581

Only one respondent so far, but that's enough to convince me that it's a
misfeature and not a bug, so perhaps we should mark 1973 as done/closed
(I'd do this myself but I don't know how).

The question for me now is how to make buffer *Async Command Output*
behave like buffer *shell*, an odd thing to be doing given that they are
both in Shell mode!

Sebastian
-- 
Emacs' AlsaPlayer - Music Without Jolts
Lightweight, full-featured and mindful of your idyllic happiness.
http://home.gna.org/eap

bug#1973: Bug in simple.el (Emacs version 22.2.1)

[Sebastian Tennant]

Quoth Sebastian Tennant <sebyte@smolny.plus.com>:
> The question for me now is how to make buffer *Async Command Output*
> behave like buffer *shell*, an odd thing to be doing given that they are
> both in Shell mode!

And I've found a solution, but it's ugly:

 (add-hook 'shell-mode-hook
           (lambda ()
             (set-process-filter proc 'comint-output-filter)))

No one likes to see the guts of a function (local variable 'proc') in
their mode hooks.

This works because shell-mode is called after the process is started.

--- shell-command snippet ---

 (with-current-buffer buffer
   (setq buffer-read-only nil)
   (erase-buffer)
   (display-buffer buffer)
   (setq default-directory directory)
   (setq proc (start-process "Shell" buffer shell-file-name
                             shell-command-switch command))
   (setq mode-line-process '(":%s"))
   (require 'shell) (shell-mode)
   (set-process-sentinel proc 'shell-command-sentinel))

--- end of snippet ---

The whole thing's a bit of a mess if you ask me.

We have two buffers (*shell* and *Async Command Output*) that both claim
to be in Shell mode, yet process output is handled completely
differently in each case.

Why is *Async Command Output* in Shell mode at all if we're not to
assume it will only be used for shell commands (that require ^M
character handling)?

Even replacing the call to shell-mode with a call to comint-mode makes
no difference to the way ^M characters are handled.  In either case the
process filter must be explicitly set to 'comint-ouput-filter.  I'd
expect something as visually arresting as mangled output to be handled
by a mode setting, but hey ho.

If it were up to me, I'd rewrite the asynchronous part of shell-command
so that make-comint-in-buffer is used to create a Comint mode buffer
called *Async Shell Command Output* and leave it at that.  After all,
the command that creates the buffer is called shell-command so let's
assume that's what the buffer is for.

I can't think of a single shell command that would depend on ^M
characters _not_ being handled in this way for its output to display
properly.  Can you think of any?

Sebastian

P.S.  Are there any buffer naming conventions in respect to
      capitalisation of words?  The lowercase 's' in *shell* has always
      struck me as odd alongside *Messages*, *Buffer List* e.t.c.

-- 
Emacs' AlsaPlayer - Music Without Jolts
Lightweight, full-featured and mindful of your idyllic happiness.
http://home.gna.org/eap

bug#1973: Bug in simple.el (Emacs version 22.2.1)

[Sebastian Tennant]

Quoth Sebastian Tennant <sebyte@smolny.plus.com>:
> No one likes to see the guts of a function (local variable 'proc') in
> their mode hooks.

This isn't so ugly.

 http://article.gmane.org/gmane.emacs.help/61630

> The whole thing's a bit of a mess if you ask me.

Apologies for the rant!  I don't know if you agree with any of it but
one thing's for sure; adding async-shell-command-filter-hook to
simple.el (and documenting it) solves the problem without changing the
implementation of shell-command in any way - a win-win situation, no?

Sebastian
-- 
Emacs' AlsaPlayer - Music Without Jolts
Lightweight, full-featured and mindful of your idyllic happiness.
http://home.gna.org/eap

bug#2103: Bug in simple.el

[Sebastian Tennant]

Hi Stefan,

Returning to the bug list, in case others have opinions on the matter...

Quoth Stefan Monnier <monnier@iro.umontreal.ca>:
>> Apologies for contacting you off-list.  I'm doing so just in case you'd
>> given up on me.  My penultimate email to the Emacs bug list was
>> something of a rant!
>>
>> Since then I think I've found the perfect solution.  A hook needs to be
>> added to simple.el providing ^M handling and we don't need to change
>> anything in shell-command.
>>
>>  http://article.gmane.org/gmane.emacs.help/61630
>
> Hooks are great for users, but for internal use they're usually
> better avoided since we can change the code directly instead.

But in this case, some users may not want ^M handling, so providing a
hook gives users the choice, no?

> E.g. in this case we want to change shell-command.

I agree that given the command is called 'shell-command' it would make
sense for ^M characters to be handled in shell-like way, i.e., no ^Ms,
in which case perhaps we should provide a spearate command for users who
_don't_ want ^M handling; 'unfiltered-shell-command' or something like
that?

Then again, adding a hook and leaving shell-command as it stands is the
simplest option and has the advantage of not breaking anything, i.e.,
no backward compatibility issues.

Seb

bug#1973: marked as done (Bug in simple.el (Emacs version 22.2.1))

[Emacs bug Tracking System]

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


Your message dated Tue, 11 Aug 2009 00:40:04 -0400
with message-id <87ocqnneu3.fsf@cyd.mit.edu>
and subject line Re: Bug in simple.el (Emacs version 22.2.1)
has caused the Emacs bug report #1973,
regarding Bug in simple.el (Emacs version 22.2.1)
to be marked as done.

This means that you claim that the problem has been dealt with.
If this is not the case it is now your responsibility to reopen the
bug report if necessary, and/or fix the problem forthwith.

(NB: If you are a system administrator and have no idea what this
message is talking about, this may indicate a serious mail system
misconfiguration somewhere. Please contact owner@emacsbugs.donarmstrong.com
immediately.)


-- 
1973: http://emacsbugs.donarmstrong.com/cgi-bin/bugreport.cgi?bug=1973
Emacs Bug Tracking System
Contact owner@emacsbugs.donarmstrong.com with problems

[-- Attachment #2: Type: message/rfc822, Size: 3785 bytes --]

[-- Attachment #2.1.1: Type: text/plain, Size: 352 bytes --]

Hello all,

Asynchronous commands called via shell-command, for example:

 (shell-command "apt-get update &")

fill the buffer *Async Shell Command Output* with Ctrl-Ms, which I'm
sure is not what's intended.

My patch (attached) fixes this by using make-comint-in-buffer, rather
than start-process, to call the asynchronous process.

HTH,

Sebastian


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2.1.2: simple.el.diff --]
[-- Type: text/x-diff, Size: 730 bytes --]

--- /usr/share/emacs/22.2/lisp/simple-patched.el	2009-01-20 19:51:29.528333079 +0000
+++ /usr/share/emacs/22.2/lisp/simple.el	2008-03-11 01:57:32.000000000 +0000
@@ -1892,11 +1892,10 @@
 		  (erase-buffer)
 		  (display-buffer buffer)
 		  (setq default-directory directory)
-		  (setq proc (get-buffer-process
-			      (make-comint-in-buffer
-			       "Shell" buffer shell-file-name nil
-			       shell-command-switch command)))
+		  (setq proc (start-process "Shell" buffer shell-file-name
+					    shell-command-switch command))
 		  (setq mode-line-process '(":%s"))
+		  (require 'shell) (shell-mode)
 		  (set-process-sentinel proc 'shell-command-sentinel)
 		  ))
 	    (shell-command-on-region (point) (point) command

[-- Attachment #3: Type: message/rfc822, Size: 1072 bytes --]

From: Chong Yidong <cyd@stupidchicken.com>
To: 1973-done@emacsbugs.donarmstrong.com
Subject: Re: Bug in simple.el (Emacs version 22.2.1)
Date: Tue, 11 Aug 2009 00:40:04 -0400
Message-ID: <87ocqnneu3.fsf@cyd.mit.edu>

Closing Bug#1973, since async-shell-command is now in the trunk.

bug#2103: marked as done (Bug in simple.el)

[Emacs bug Tracking System]

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


Your message dated Tue, 11 Aug 2009 00:40:04 -0400
with message-id <87ocqnneu3.fsf@cyd.mit.edu>
and subject line Re: Bug in simple.el (Emacs version 22.2.1)
has caused the Emacs bug report #1973,
regarding Bug in simple.el
to be marked as done.

This means that you claim that the problem has been dealt with.
If this is not the case it is now your responsibility to reopen the
bug report if necessary, and/or fix the problem forthwith.

(NB: If you are a system administrator and have no idea what this
message is talking about, this may indicate a serious mail system
misconfiguration somewhere. Please contact owner@emacsbugs.donarmstrong.com
immediately.)


-- 
1973: http://emacsbugs.donarmstrong.com/cgi-bin/bugreport.cgi?bug=1973
Emacs Bug Tracking System
Contact owner@emacsbugs.donarmstrong.com with problems

[-- Attachment #2: Type: message/rfc822, Size: 3960 bytes --]

From: Sebastian Tennant <sebyte@smolny.plus.com>
To: bug-gnu-emacs@gnu.org
Subject: Re: Bug in simple.el
Date: Thu, 29 Jan 2009 07:06:22 +0000
Message-ID: <ljsu8uc1.fsf@vps203.linuxvps.org>

Hi Stefan,

Returning to the bug list, in case others have opinions on the matter...

Quoth Stefan Monnier <monnier@iro.umontreal.ca>:
>> Apologies for contacting you off-list.  I'm doing so just in case you'd
>> given up on me.  My penultimate email to the Emacs bug list was
>> something of a rant!
>>
>> Since then I think I've found the perfect solution.  A hook needs to be
>> added to simple.el providing ^M handling and we don't need to change
>> anything in shell-command.
>>
>>  http://article.gmane.org/gmane.emacs.help/61630
>
> Hooks are great for users, but for internal use they're usually
> better avoided since we can change the code directly instead.

But in this case, some users may not want ^M handling, so providing a
hook gives users the choice, no?

> E.g. in this case we want to change shell-command.

I agree that given the command is called 'shell-command' it would make
sense for ^M characters to be handled in shell-like way, i.e., no ^Ms,
in which case perhaps we should provide a spearate command for users who
_don't_ want ^M handling; 'unfiltered-shell-command' or something like
that?

Then again, adding a hook and leaving shell-command as it stands is the
simplest option and has the advantage of not breaking anything, i.e.,
no backward compatibility issues.

Seb





[-- Attachment #3: Type: message/rfc822, Size: 1072 bytes --]

From: Chong Yidong <cyd@stupidchicken.com>
To: 1973-done@emacsbugs.donarmstrong.com
Subject: Re: Bug in simple.el (Emacs version 22.2.1)
Date: Tue, 11 Aug 2009 00:40:04 -0400
Message-ID: <87ocqnneu3.fsf@cyd.mit.edu>

Closing Bug#1973, since async-shell-command is now in the trunk.