GNU bug report logs - #7899
23.2.91; Unsatisfactory interaction between shell-mode-hook and comint-read-input-ring

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 7899 in the body.
You can then email your comments to 7899 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Reuben Thomas <rrt <at> sc3d.org>
To: bug-gnu-emacs <at> gnu.org
Subject: 23.2.91; Unsatisfactory interaction between shell-mode-hook and
	comint-read-input-ring
Date: Sun, 23 Jan 2011 19:12:45 +0000

shell-mode runs comint-read-input-ring. This is in a way unfortunate,
since this means that if you want to change the parameters of
comint-input-ring in shell-mode-hook, then you’re already too late.

I have the following in my .emacs:

(add-hook 'shell-mode-hook
          (lambda ()
            (let ((n (getenv "HISTSIZE")))
              (if n (progn (setq comint-input-ring-size (string-to-number n))
                           (comint-read-input-ring t))))))

This is not appropriate for comint-mode-hook, since HISTSIZE is
shell-specific (really, it’s bash-specific, so I should be testing that
I’m running bash). But now I have to run comint-read-input-ring again.

Perhaps shell-mode should instead append comint-read-input-ring to
shell-mode-hook if it’s not already there, or some such malarky?

There is one further thing that is puzzling me about the situation: I
have quite a large ~/.bash_history (about 200,000 lines). When I ran
shell-mode without first setting comint-input-ring-size correctly, it
was truncated to 500 lines when I killed the shell-mode buffer (as I
would expect). However, after setting comint-input-ring-size to HISTSIZE
(which is 400,000), ~/.bash_history was no longer translated _even
though, as I was not yet running comint-read-input-ring in my shell-mode
hook, only 500 entries were being read_. This seems to contradict the
docstring of comint-write-input-ring, which says that it overwrites
comint-input-ring-file-name if comint-input-ring is non-empty. (However,
it is correct behavior for my shell: I have set shopt histappend. But
this is surely a coincidence, as bash will update the history file
before Emacs, so Emacs should “win”, right?)


In GNU Emacs 23.2.91.4 (i686-pc-linux-gnu, GTK+ Version 2.22.0)
 of 2011-01-13 on canta
Windowing system distributor `The X.Org Foundation', version 11.0.10900000
Important settings:
  value of $LC_ALL: nil
  value of $LC_COLLATE: nil
  value of $LC_CTYPE: nil
  value of $LC_MESSAGES: nil
  value of $LC_MONETARY: nil
  value of $LC_NUMERIC: nil
  value of $LC_TIME: nil
  value of $LANG: en_GB.UTF-8
  value of $XMODIFIERS: nil
  locale-coding-system: utf-8-unix
  default enable-multibyte-characters: t

Major mode: Shell

Minor modes in effect:
  shell-dirtrack-mode: t
  recentf-mode: t
  show-paren-mode: t
  savehist-mode: t
  minibuffer-electric-default-mode: t
  iswitchb-mode: t
  icomplete-mode: t
  global-whitespace-mode: t
  global-auto-revert-mode: t
  desktop-save-mode: t
  yas/global-mode: t
  mouse-wheel-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  column-number-mode: t
  line-number-mode: t
  transient-mark-mode: t

Recent input:
M-x s h e l l <return> C-h v c o m i n t - i n p u 
t - r i n g <return> M-> M-< M-< C-x k <return> a M-p 
M-p M-p M-p M-p M-p M-p M-p M-p M-p M-p M-n M-n M-n 
M-n M-n M-n M-n M-n M-n M-n M-n M-n M-n M-n M-n M-n 
M-n M-n M-n M-n M-n M-n M-n M-n M-n M-n M-n M-n M-n 
M-n M-n M-n M-n M-n M-n M-n M-n M-n M-n M-n M-n M-n 
M-n M-n M-n M-n M-n M-n M-n <M-backspace> <M-backspace> 
<return> . / M-p M-p M-p M-p M-p M-p M-p M-p M-p M-p 
M-p M-p M-p M-p M-p C-a C-k <return> <return> M-x r 
e p o r t - e m a c s - b u g <return>

Recent messages:
History item: 897
History item: 900
History item: 912
History item: 913
History item: 1202
History item: 1203
History item: 1206
History item: 1399
History item: 1400
History item: 1404

Load-path shadows:
/home/rrt/.emacs.d/elpa/ruby-mode-1.1/ruby-mode hides /usr/share/emacs-snapshot/site-lisp/ruby1.8-elisp/ruby-mode
/home/rrt/local/share/emacs/site-lisp/popup hides /usr/local/share/emacs/23.2.91/site-lisp/auto-complete/popup
/home/rrt/local/share/emacs/site-lisp/fuzzy hides /usr/local/share/emacs/23.2.91/site-lisp/auto-complete/fuzzy
/home/rrt/.emacs.d/elpa/css-mode-1.0/css-mode hides /usr/local/share/emacs/23.2.91/site-lisp/css-mode/css-mode
/home/rrt/.emacs.d/elpa/dictionary-1.8.7/link hides /usr/local/share/emacs/23.2.91/site-lisp/dictionary-el/link
/home/rrt/.emacs.d/elpa/dictionary-1.8.7/connection hides /usr/local/share/emacs/23.2.91/site-lisp/dictionary-el/connection
/home/rrt/.emacs.d/elpa/dictionary-1.8.7/dictionary-init hides /usr/local/share/emacs/23.2.91/site-lisp/dictionary-el/dictionary-init
/home/rrt/.emacs.d/elpa/dictionary-1.8.7/dictionary hides /usr/local/share/emacs/23.2.91/site-lisp/dictionary-el/dictionary
/home/rrt/.emacs.d/elpa/css-mode-1.0/css-mode hides /usr/local/share/emacs/23.2.91/lisp/textmodes/css-mode
/home/rrt/.emacs.d/elpa/ruby-mode-1.1/ruby-mode hides /usr/local/share/emacs/23.2.91/lisp/progmodes/ruby-mode
/home/rrt/.emacs.d/elpa/css-mode-1.0/css-mode hides /usr/share/emacs/site-lisp/css-mode/css-mode
/usr/local/share/emacs/23.2.91/site-lisp/auctex/tex-info hides /usr/share/emacs/site-lisp/auctex/tex-info
/usr/local/share/emacs/23.2.91/site-lisp/auctex/context-nl hides /usr/share/emacs/site-lisp/auctex/context-nl
/usr/local/share/emacs/23.2.91/site-lisp/auctex/context-en hides /usr/share/emacs/site-lisp/auctex/context-en
/usr/local/share/emacs/23.2.91/site-lisp/auctex/latex hides /usr/share/emacs/site-lisp/auctex/latex
/usr/local/share/emacs/23.2.91/site-lisp/auctex/tex-mik hides /usr/share/emacs/site-lisp/auctex/tex-mik
/usr/local/share/emacs/23.2.91/site-lisp/dictionary-el/lpath hides /usr/share/emacs/site-lisp/auctex/lpath
/usr/local/share/emacs/23.2.91/site-lisp/auctex/tex-buf hides /usr/share/emacs/site-lisp/auctex/tex-buf
/usr/local/share/emacs/23.2.91/site-lisp/auctex/tex-jp hides /usr/share/emacs/site-lisp/auctex/tex-jp
/usr/local/share/emacs/23.2.91/site-lisp/auctex/tex-bar hides /usr/share/emacs/site-lisp/auctex/tex-bar
/usr/local/share/emacs/23.2.91/site-lisp/auctex/tex hides /usr/share/emacs/site-lisp/auctex/tex
/usr/local/share/emacs/23.2.91/site-lisp/auctex/multi-prompt hides /usr/share/emacs/site-lisp/auctex/multi-prompt
/usr/local/share/emacs/23.2.91/site-lisp/auctex/tex-fptex hides /usr/share/emacs/site-lisp/auctex/tex-fptex
/usr/local/share/emacs/23.2.91/site-lisp/auctex/tex-font hides /usr/share/emacs/site-lisp/auctex/tex-font
/usr/local/share/emacs/23.2.91/site-lisp/auctex/tex-fold hides /usr/share/emacs/site-lisp/auctex/tex-fold
/usr/local/share/emacs/23.2.91/site-lisp/auctex/texmathp hides /usr/share/emacs/site-lisp/auctex/texmathp
/usr/local/share/emacs/23.2.91/site-lisp/auctex/context hides /usr/share/emacs/site-lisp/auctex/context
/usr/local/share/emacs/23.2.91/site-lisp/auctex/font-latex hides /usr/share/emacs/site-lisp/auctex/font-latex
/usr/local/share/emacs/23.2.91/site-lisp/auctex/bib-cite hides /usr/share/emacs/site-lisp/auctex/bib-cite
/usr/local/share/emacs/23.2.91/site-lisp/auctex/toolbar-x hides /usr/share/emacs/site-lisp/auctex/toolbar-x
/usr/local/share/emacs/23.2.91/site-lisp/auctex/tex-style hides /usr/share/emacs/site-lisp/auctex/tex-style

Features:
(shadow sort mail-extr message sendmail ecomplete rfc822 mml mml-sec
password-cache mm-decode mm-bodies mm-encode mailcap mail-parse rfc2231
rfc2047 rfc2045 qp ietf-drums mailabbrev nnheader gnus-util netrc
time-date mm-util mail-prsvr gmm-utils mailheader canlock sha1 hex-util
hashcash mail-utils emacsbug vc-git pp help-mode view ansi-color shell
jka-compr info face-remap filladapt flyspell completing-help recentf
tree-widget wid-edit uniquify paren savehist minibuf-eldef iswitchb
icomplete whitespace autorevert time cus-start cus-load desktop server
php-mode etags cc-langs cc-mode cc-fonts cc-menus cc-cmds cc-styles
cc-align cc-engine cc-vars cc-defs speedbar sb-image ezimage dframe
lua-mode regexp-opt comint ring ropemacs pymacs etags-update
smart-quotes ispell auto-dictionary-autoloads css-mode-autoloads
dictionary-autoloads diff-git-autoloads dired-isearch-autoloads
full-ack-autoloads guess-style-autoloads http-post-simple-autoloads
js2-mode-autoloads magit-autoloads mv-shell-autoloads
ruby-mode-autoloads package reporter advice advice-preload yasnippet
help-fns derived edmacro kmacro easymenu assoc cl cl-19 muse-autoloads
emacs-goodies-el emacs-goodies-custom emacs-goodies-loaddefs easy-mmode
bbdb-autoloads preview-latex tex-site auto-loads tooltip ediff-hook
vc-hooks lisp-float-type mwheel x-win x-dnd font-setting tool-bar dnd
fontset image fringe lisp-mode register page menu-bar rfn-eshadow timer
select scroll-bar mldrag mouse jit-lock font-lock syntax facemenu
font-core frame cham georgian utf-8-lang misc-lang vietnamese tibetan
thai tai-viet lao korean japanese hebrew greek romanian slovak czech
european ethiopic indian cyrillic chinese case-table epa-hook
jka-cmpr-hook help simple abbrev loaddefs button minibuffer faces
cus-face files text-properties overlay md5 base64 format env code-pages
mule custom widget hashtable-print-readable backquote
make-network-process dbusbind system-font-setting font-render-setting
gtk x-toolkit x multi-tty emacs)

-- 
http://rrt.sc3d.org/

Message #8 received at 7899 <at> debbugs.gnu.org (full text, mbox):

From: Pete Beardmore <pete.beardmore <at> msn.com>
To: 7899 <at> debbugs.gnu.org
Subject: bug#7899 Unsatisfactory interaction between shell-mode-hook and
	comint-read-input-ring
Date: Mon, 27 May 2013 08:44:25 +0100

[Message part 1 (text/plain, inline)]

[patches against bzr master attached (hopefully!)]

hi,

i believe this is a bug in comint-read-input-ring. shell-mode sets a  
buffer-local version of comint-read-input-size which is effectively  
ignored due to comint-read-input-ring's use of '(with-temp-buffer ...'  
. i've lost ~40000 line bash history on more than one occasion over  
the last several years and am elated to have finally pinned the  
problem on something

(very loosely) related to this issue is the question of why the  
default of 'comint-input-history-ignore' is set to anything at all?  
it's currently "^#", and therefore without having pro-actively made  
any changes to their emacs setup, a user's shell history (for  
instance) doesn't emerge unscathed from a trip through comint if it  
contains comments. if modifying this default touches too many other  
comint uses, perhaps an override in shell-mode.el?

cheers,
Pete

[0001.comint_.ensure.buffer.local.comint-input-ring-read-size.variable.is.visible.throughout.input-read-ring.diff (text/x-patch, attachment)]

[0001.comint_.don't.strip.anything.by.default.on.comint-input-ring-read.diff (text/x-patch, attachment)]

Message #11 received at 7899 <at> debbugs.gnu.org (full text, mbox):

From: Pete Beardmore <pete.beardmore <at> msn.com>
To: 7899 <at> debbugs.gnu.org
Subject: Re: bug#7899: Unsatisfactory interaction between shell-mode-hook
	and comint-read-input-ring
Date: Thu, 30 May 2013 13:55:03 +0100

[Message part 1 (text/plain, inline)]

Quoting Pete Beardmore <pete.beardmore <at> msn.com>:

> [patches against bzr master attached (hopefully!)]
>
> hi,
>
> i believe this is a bug in comint-read-input-ring. shell-mode sets a  
> buffer-local version of comint-read-input-size which is effectively  
> ignored due to comint-read-input-ring's use of '(with-temp-buffer  
> ...' . i've lost ~40000 line bash history on more than one occasion  
> over the last several years and am elated to have finally pinned the  
> problem on something
>
> (very loosely) related to this issue is the question of why the  
> default of 'comint-input-history-ignore' is set to anything at all?  
> it's currently "^#", and therefore without having pro-actively made  
> any changes to their emacs setup, a user's shell history (for  
> instance) doesn't emerge unscathed from a trip through comint if it  
> contains comments. if modifying this default touches too many other  
> comint uses, perhaps an override in shell-mode.el?
>
> cheers,
> Pete

[patches attached superseed previous patches]

hello,

patch 1:
i've extended the original fix for ignoring buffer-local variables to  
incorporate 'comint-input-ring-separator',  
'comint-input-history-ignore' and 'comint-input-ignoredups' vars which  
suffered from the same issue

patch 2:
as before, but note that this request to change the default  
'comint-input-history-ignore' from '^#' to '' exposed another bug in  
the 'comint-read-input-ring' code. see patch 3

patch 3:
if 'comint-input-history-ignore' is set to "" (not 'nil' as we're  
using string-match), string-match will always return 0 ..and as this  
isn't nil, all potential items are dropped as they match the ignore  
string. i'll leave 'patch 2' as a request, but the fix for this bug is  
a necessity i think as there's nothing stopping users setting ignore  
to "" as it stands, and that causes issues

patch 4, the ignore-dupes functionality didn't work at all*. the  
comparison of the current item (to be placed into the ring) was being  
made against (ring-ref ring 0) ..which is static, and not the last  
item we added as is needed here. the docs on  
'ring-insert-at-beginning'/'ring-insert'/'ring-ref' would confuse  
anyone on first glance (in defense of whoever slipped here initially)

*it does 'work' if the only dupes in the file are all adjacent and  
equal to the last item

cheers,
Pete

ps. there's still a nasty mix of tabs/space formatting in  
'comint-read-input-ring'. i harmonised only the block i touched

[0001.comint_.ensure.buffer.local.comint-input-ring-_.variables.are.visible.through.input-read-ring.logic.diff (text/x-patch, attachment)]

[0002.comint_.don't.strip.anything.by.default.on.comint-input-ring-read.diff (text/x-patch, attachment)]

[0003.comint_.don't.match.an.empty.ignore.string.diff (text/x-patch, attachment)]

[0004.comint_.fix.ignore-dupe.functionality.diff (text/x-patch, attachment)]

Message #16 received at 7899 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Pete Beardmore <pete.beardmore <at> msn.com>
Cc: 7899 <at> debbugs.gnu.org
Subject: Re: bug#7899: 23.2.91; Unsatisfactory interaction between
 shell-mode-hook and comint-read-input-ring
Date: Mon, 10 May 2021 13:16:50 +0200

Pete Beardmore <pete.beardmore <at> msn.com> writes:

> patch 1:
> i've extended the original fix for ignoring buffer-local variables to
> incorporate 'comint-input-ring-separator',
> 'comint-input-history-ignore' and 'comint-input-ignoredups' vars which
> suffered from the same issue

Thanks for the patch -- it seems like it didn't receive any attention at
the time (seven years ago), unfortunately.

And if I'm reading the commit logs correctly, this was fixed in 2019 in
a slightly different way in commit 9b54d2b66e1e, so I'm closing this bug
report.  If there's still something to be worked on here, please respond
to the debbugs address, and we'll reopne.

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

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.