bug#19812: 24.4; suggest `shell-mode' not interactive

bug#19812: 24.4; suggest `shell-mode' not interactive

[Kevin Ryde]

Severity: wishlist

The `shell-mode' function is interactive, but I think perhaps it should
not be since it is not designed for use as M-x shell-mode in an
arbitrary buffer, but only after the `shell' function makes various
setups.

I struck this when a brain fade confused me between shell-command,
shell-mode and shell and I did M-x shell-mode in a file buffer and
nearly made a big mess.  The problem is only seen when shell.el has been
loaded (for any reason) since shell-mode is not autoloaded.

Similar could apply to other special modes, maybe even to most modes
derived from `special-mode'.


In GNU Emacs 24.4.1 (i586-pc-linux-gnu, X toolkit, Xaw3d scroll bars)
 of 2014-12-20 on brahms, modified by Debian
Windowing system distributor `The X.Org Foundation', version 11.0.11602901
Configured using:
 `configure --build i586-linux-gnu --prefix=/usr
 --sharedstatedir=/var/lib --libexecdir=/usr/lib
 --localstatedir=/var/lib --infodir=/usr/share/info
 --mandir=/usr/share/man --with-pop=yes
 --enable-locallisppath=/etc/emacs24:/etc/emacs:/usr/local/share/emacs/24.4/site-lisp:/usr/local/share/emacs/site-lisp:/usr/share/emacs/24.4/site-lisp:/usr/share/emacs/site-lisp
 --build i586-linux-gnu --prefix=/usr --sharedstatedir=/var/lib
 --libexecdir=/usr/lib --localstatedir=/var/lib
 --infodir=/usr/share/info --mandir=/usr/share/man --with-pop=yes
 --enable-locallisppath=/etc/emacs24:/etc/emacs:/usr/local/share/emacs/24.4/site-lisp:/usr/local/share/emacs/site-lisp:/usr/share/emacs/24.4/site-lisp:/usr/share/emacs/site-lisp
 --with-x=yes --with-x-toolkit=lucid --with-toolkit-scroll-bars
 --without-gconf --without-gsettings 'CFLAGS=-g -O2
 -fstack-protector-strong -Wformat -Werror=format-security -Wall'
 CPPFLAGS=-D_FORTIFY_SOURCE=2 LDFLAGS=-Wl,-z,relro'

bug#19812: 24.4; suggest `shell-mode' not interactive

[Lars Ingebrigtsen]

Kevin Ryde <user42_kevin@yahoo.com.au> writes:

> The `shell-mode' function is interactive, but I think perhaps it should
> not be since it is not designed for use as M-x shell-mode in an
> arbitrary buffer, but only after the `shell' function makes various
> setups.
>
> I struck this when a brain fade confused me between shell-command,
> shell-mode and shell and I did M-x shell-mode in a file buffer and
> nearly made a big mess.  The problem is only seen when shell.el has been
> loaded (for any reason) since shell-mode is not autoloaded.
>
> Similar could apply to other special modes, maybe even to most modes
> derived from `special-mode'.

As you point out, there's a lot of modes that only make sense in certain
limited contexts, and having them be interactive isn't very helpful.
I'm not sure it's that much of a problem in general, but it is a
particular an issue with `shell-mode', since I'd guess that a lot of
people type that instead of `M-x shell-script-mode' when starting to
write new shell scripts.

The following fixes this particular issue.  Would something like this be
welcome?

A different fix would be to extend defined-derived-mode to take a
:noninteractive flag to just not include the `(interactive)' in the
defun.  It looks fairly straightforward to implement -- is that a better
idea, perhaps?

(That is, if we want to fix this at all.)

diff --git a/lisp/shell.el b/lisp/shell.el
index 2914d1d2c8..ba7515e7ba 100644
--- a/lisp/shell.el
+++ b/lisp/shell.el
@@ -553,6 +553,8 @@ shell-mode
 `comint-scroll-to-bottom-on-input' and `comint-scroll-to-bottom-on-output'
 control whether input and output cause the window to scroll to the end of the
 buffer."
+  (when (called-interactively-p 'any)
+    (error "Can't be called interactively; did you mean `shell-script-mode' instead?"))
   (setq comint-prompt-regexp shell-prompt-pattern)
   (shell-completion-vars)
   (setq-local paragraph-separate "\\'")


-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#19812: 24.4; suggest `shell-mode' not interactive

[Kevin Ryde]

Lars Ingebrigtsen <larsi@gnus.org> writes:
>
> A different fix would be to extend defined-derived-mode to take a
> :noninteractive flag to just not include the `(interactive)' in the
> defun.  It looks fairly straightforward to implement -- is that a better
> idea, perhaps?

Maybe :interactive, just to write it in the positive sense.  The default
must be t for compatibility.  A couple of words in the docs could note
that special modes usually should be nil.

Perhaps special-mode itself would be the second customer for this.
I presume special-mode is not intended for interactive use, and it
doesn't do much helpful (set buffer-read-only).

bug#19812: 24.4; suggest `shell-mode' not interactive

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Date: Fri, 02 Aug 2019 21:52:17 +0200
> Cc: 19812@debbugs.gnu.org
> 
> As you point out, there's a lot of modes that only make sense in certain
> limited contexts, and having them be interactive isn't very helpful.
> I'm not sure it's that much of a problem in general, but it is a
> particular an issue with `shell-mode', since I'd guess that a lot of
> people type that instead of `M-x shell-script-mode' when starting to
> write new shell scripts.
> 
> The following fixes this particular issue.  Would something like this be
> welcome?

Are there no valid use cases where invoking shell-mode interactively
makes sense?  If there are such use cases, we cannot disallow them
now.

bug#19812: 24.4; suggest `shell-mode' not interactive

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> Are there no valid use cases where invoking shell-mode interactively
> makes sense?  If there are such use cases, we cannot disallow them
> now.

I can't think of any cases where you'd call `shell-mode'
interactively.

Hm.  Perhaps if you've started a comint buffer using another mode and
then wants to switch to `shell-mode'?  But I doubt that that's a thing.

Perhaps somebody else will chime in to say whether they've ever said
`M-x shell-mode' and it wasn't a mistake.  :-)

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#19812: 24.4; suggest `shell-mode' not interactive

[Lars Ingebrigtsen]

Kevin Ryde <user42_kevin@yahoo.com.au> writes:

> Maybe :interactive, just to write it in the positive sense.  The default
> must be t for compatibility.

That makes sense, and is easy to implement in this case.  We usually
treat the absence of a parameter the same as nil (since Emacs Lisp isn't
Common Lisp), but we don't have to do that here.

> A couple of words in the docs could note that special modes usually
> should be nil.

True, but I don't think we need to limit the interactivity here, though.
When there's little chance of confusion, having the mode be interactive
isn't that big a deal (and might be useful for somebody, sometime).

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#19812: 24.4; suggest `shell-mode' not interactive

[Juri Linkov]

> Perhaps somebody else will chime in to say whether they've ever said
> `M-x shell-mode' and it wasn't a mistake.  :-)

It might be useful to visit log files previously saved from shell buffers,
like compilation-mode adds such local-variable prop-line:

  -*- mode: compilation -*-

so compilation log saved to files could be visited later.

But one problem with using

  -*- mode: shell -*-

in files saved from previous shells is that it doesn't highlight prompts -
thus it lacks a very important feature of shell-mode.

bug#19812: 24.4; suggest `shell-mode' not interactive

[Kevin Ryde]

Juri Linkov <juri@linkov.net> writes:
>
> It might be useful to visit log files previously saved from shell buffers,
> like compilation-mode adds such local-variable prop-line:
>
>   -*- mode: compilation -*-

I suppose the difference is compilation-mode does useful things when a
compilation has finished, whereas shell-mode and similar would be more
or less entirely for interaction.

I've had file buffers in compilation-mode for stepping error lines in
the way you say.  The only thing I've been tricked by is accidentally
"g" recompile which replaces the file text -- in a sensible way, but for
me usually not what I meant to do :-).



-- 
Monotremes oviparous, ovum meroblastic.  -- Caldwell

bug#19812: 24.4; suggest `shell-mode' not interactive

[Juri Linkov]

>> It might be useful to visit log files previously saved from shell buffers,
>> like compilation-mode adds such local-variable prop-line:
>>
>>   -*- mode: compilation -*-
>
> I suppose the difference is compilation-mode does useful things when a
> compilation has finished, whereas shell-mode and similar would be more
> or less entirely for interaction.

It makes sense to visit a previously saved shell buffer only to
see highlighted prompts and command lines (that shell-mode
currently is unable to do).

> I've had file buffers in compilation-mode for stepping error lines in
> the way you say.  The only thing I've been tricked by is accidentally
> "g" recompile which replaces the file text -- in a sensible way, but for
> me usually not what I meant to do :-).

Maybe "g" should check if the compilation buffer is visiting a file,
and not to recompile in this case.

bug#19812: 24.4; suggest `shell-mode' not interactive

[Kevin Ryde]

Juri Linkov <juri@linkov.net> writes:
>
> Maybe "g" should check if the compilation buffer is visiting a file,
> and not to recompile in this case.

I don't know if that'd be too strict, and yet some query-the-user could
be annoying when you know what you're doing - such as having only just
this moment saved a compile buffer to a file.

The scariest bit for me was undo is disabled, so you can't go back when
you realize your mistake.  I suspect making undo the right thing would
be hard, but in a file buffer that's what you reach for first on strange
changes, so it's a bit off-putting.

bug#19812: 24.4; suggest `shell-mode' not interactive

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

>> As you point out, there's a lot of modes that only make sense in certain
>> limited contexts, and having them be interactive isn't very helpful.
>> I'm not sure it's that much of a problem in general, but it is a
>> particular an issue with `shell-mode', since I'd guess that a lot of
>> people type that instead of `M-x shell-script-mode' when starting to
>> write new shell scripts.
>> 
>> The following fixes this particular issue.  Would something like this be
>> welcome?
>
> Are there no valid use cases where invoking shell-mode interactively
> makes sense?  If there are such use cases, we cannot disallow them
> now.

If I read the comments correctly, nobody had a use case for using `M-x
shell-mode'.  It was proposed that it could possibly be useful for
working in log files and the like, but that shell-mode doesn't currently
actually support that, so I went ahead and applied the patch to disable
the interactive use of the mode.

If this leads to problems for anybody, feel free to revert the commit.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#19812: 24.4; suggest `shell-mode' not interactive

[Juri Linkov]

> If I read the comments correctly, nobody had a use case for using `M-x
> shell-mode'.  It was proposed that it could possibly be useful for
> working in log files and the like, but that shell-mode doesn't currently
> actually support that, so I went ahead and applied the patch to disable
> the interactive use of the mode.

I confirm that `M-x shell-mode' can't be used in saved log files.  I tried:

  (defvar shell-log-font-lock-keywords
    ;; `shell-prompt-pattern' can't be used: it finds too many false matches
    '(("^\\([^#$%>\12]*@[^#$%>\12]*:[^#$%>\12]*[#$%>] *\\)\\(.*\\)$"
       (1 'comint-highlight-prompt)
       (2 'comint-highlight-input)))
    "Shell prompts to highlight in Shell Log mode.")

  (define-derived-mode shell-log-mode shell-mode "Shell-Log"
    "Font-lock for shell logs."
    (put 'shell-log-mode 'mode-class nil)
    (setq-local font-lock-defaults '(shell-log-font-lock-keywords t)))

  (add-to-list 'auto-mode-alist '("\\.log\\'" . shell-log-mode))

but `shell-prompt-pattern' matches too many false positives, and
replacing it with a customized regexp is too ad-hoc and unreliable.