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

Previous Next

Full log

View this message in rfc822 format

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: 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)]

Previous Next

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