[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

bug#7899: Unsatisfactory interaction between shell-mode-hook and comint-

From: Pete Beardmore
Subject: bug#7899: Unsatisfactory interaction between shell-mode-hook and comint-read-input-ring
Date: Thu, 30 May 2013 13:55:03 +0100
User-agent: Internet Messaging Program (IMP) H5 (6.0.4)

Quoting Pete Beardmore <address@hidden>:

[patches against bzr master attached (hopefully!)]


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?


[patches attached superseed previous patches]


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


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

Attachment: txt2LyNp5C7PC.txt
Description: Text Data

Attachment: txtfXMq6yxuU_.txt
Description: Text Data

Attachment: 0003.comint_.don't.match.an.empty.ignore.string.diff
Description: Text Data

Attachment: 0004.comint_.fix.ignore-dupe.functionality.diff
Description: Text Data

reply via email to

[Prev in Thread] Current Thread [Next in Thread]