From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Pete Beardmore Newsgroups: gmane.emacs.bugs Subject: bug#7899: Unsatisfactory interaction between shell-mode-hook and comint-read-input-ring Date: Thu, 30 May 2013 13:55:03 +0100 Message-ID: References: <87y66b3042.fsf@sc3d.org> NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=_vtE9f0MKYXtD35YXUxzlpw4" X-Trace: ger.gmane.org 1369918621 29464 80.91.229.3 (30 May 2013 12:57:01 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Thu, 30 May 2013 12:57:01 +0000 (UTC) To: 7899@debbugs.gnu.org Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Thu May 30 14:57:00 2013 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by plane.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1Ui2Pb-0002Gc-T9 for geb-bug-gnu-emacs@m.gmane.org; Thu, 30 May 2013 14:57:00 +0200 Original-Received: from localhost ([::1]:34355 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Ui2Pb-0004Qj-KQ for geb-bug-gnu-emacs@m.gmane.org; Thu, 30 May 2013 08:56:59 -0400 Original-Received: from eggs.gnu.org ([208.118.235.92]:48328) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Ui2PK-0004Dk-Og for bug-gnu-emacs@gnu.org; Thu, 30 May 2013 08:56:57 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Ui2PC-0005ya-Jy for bug-gnu-emacs@gnu.org; Thu, 30 May 2013 08:56:42 -0400 Original-Received: from debbugs.gnu.org ([140.186.70.43]:53428) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Ui2PC-0005yP-D0 for bug-gnu-emacs@gnu.org; Thu, 30 May 2013 08:56:34 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.72) (envelope-from ) id 1Ui2Qd-0005F0-5Z for bug-gnu-emacs@gnu.org; Thu, 30 May 2013 08:58:03 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Pete Beardmore Original-Sender: debbugs-submit-bounces@debbugs.gnu.org Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Thu, 30 May 2013 12:58:03 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 7899 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: Original-Received: via spool by 7899-submit@debbugs.gnu.org id=B7899.136991862419970 (code B ref 7899); Thu, 30 May 2013 12:58:03 +0000 Original-Received: (at 7899) by debbugs.gnu.org; 30 May 2013 12:57:04 +0000 Original-Received: from localhost ([127.0.0.1]:41784 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.72) (envelope-from ) id 1Ui2Pa-0005Bj-Hw for submit@debbugs.gnu.org; Thu, 30 May 2013 08:57:03 -0400 Original-Received: from blu0-omc1-s31.blu0.hotmail.com ([65.55.116.42]:34541) by debbugs.gnu.org with esmtp (Exim 4.72) (envelope-from ) id 1Ui2PL-0005Al-7U for 7899@debbugs.gnu.org; Thu, 30 May 2013 08:56:51 -0400 Original-Received: from BLU0-SMTP7 ([65.55.116.7]) by blu0-omc1-s31.blu0.hotmail.com with Microsoft SMTPSVC(6.0.3790.4675); Thu, 30 May 2013 05:55:06 -0700 X-EIP: [xhDw84lSxC46ruYMRCgtDHoi2x1NJLz/] X-Originating-Email: [pete.beardmore@msn.com] Original-Received: from elservo.lemondedelabarbe ([93.97.95.47]) by BLU0-SMTP7.phx.gbl over TLS secured channel with Microsoft SMTPSVC(6.0.3790.4675); Thu, 30 May 2013 05:55:05 -0700 Original-Received: by elservo.lemondedelabarbe (Postfix, from userid 80) id 35BAB140C6D; Thu, 30 May 2013 13:55:04 +0100 (BST) Original-Received: from elbeardo.lemondedelabarbe (elbeardo.lemondedelabarbe [10.0.0.5]) by elservo.lemondedelabarbe (Horde Framework) with HTTP; Thu, 30 May 2013 13:55:03 +0100 In-Reply-To: User-Agent: Internet Messaging Program (IMP) H5 (6.0.4) X-OriginalArrivalTime: 30 May 2013 12:55:05.0784 (UTC) FILETIME=[E8543B80:01CE5D34] X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.13 Precedence: list X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6.x X-Received-From: 140.186.70.43 X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Original-Sender: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.bugs:74656 Archived-At: --=_vtE9f0MKYXtD35YXUxzlpw4 Content-Type: text/plain; charset="UTF-8"; format=flowed; delsp=Yes Content-Disposition: inline Quoting Pete Beardmore : > [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 --=_vtE9f0MKYXtD35YXUxzlpw4 Content-Type: text/x-patch; charset="us-ascii"; name*0="0001.comint_.ensure.buffer.local.comint-input-ring-_.variables.are.v"; name*1="isible.through.input-read-ring.logic.diff" Content-Disposition: attachment; filename*0= "0001.comint_.ensure.buffer.local.comint-input-ring-_.variables.a"; filename*1="re.visible.through.input-read-ring.logic.diff"; size=3199 #------------------------------------------------------------ #revno: 112736 #committer: Pete Beardmore #branch nick: bzr #timestamp: Thu 2013-05-30 13:17:39 +0100 #message: # comint: ensure buffer local comint-input-ring-* variables are visible through input-read-ring logic === modified file 'lisp/comint.el' --- lisp/comint.el 2013-05-25 02:40:33 +0000 +++ lisp/comint.el 2013-05-30 12:17:39 +0000 @@ -938,33 +938,33 @@ ;; to huge numbers. Don't allocate a huge ring right ;; away; there might not be that much history. (ring-size (min 1500 comint-input-ring-size)) - (ring (make-ring ring-size))) - (with-temp-buffer - (insert-file-contents file) - ;; Save restriction in case file is already visited... - ;; Watch for those date stamps in history files! - (goto-char (point-max)) - (let (start end history) - (while (and (< count comint-input-ring-size) - (re-search-backward comint-input-ring-separator - nil t) - (setq end (match-beginning 0))) - (setq start - (if (re-search-backward comint-input-ring-separator - nil t) - (match-end 0) - (point-min))) - (setq history (buffer-substring start end)) - (goto-char start) - (when (and (not (string-match comint-input-history-ignore - history)) - (or (null comint-input-ignoredups) - (ring-empty-p ring) - (not (string-equal (ring-ref ring 0) - history)))) + (ring-size-max (max 1500 comint-input-ring-size)) + (ring (make-ring ring-size)) + (ring-separator comint-input-ring-separator) + (ignore comint-input-history-ignore) + (ignoredups comint-input-ignoredups)) + (with-temp-buffer + (insert-file-contents file) + ;; Save restriction in case file is already visited... + ;; Watch for those date stamps in history files! + (goto-char (point-max)) + (let (start end history) + (while (and (< count ring-size-max) + (re-search-backward ring-separator nil t) + (setq end (match-beginning 0))) + (setq start + (if (re-search-backward ring-separator nil t) + (match-end 0) + (point-min))) + (setq history (buffer-substring start end)) + (goto-char start) + (when (and (not (string-match ignore history)) + (or (null ignoredups) + (ring-empty-p ring) + (not (string-equal (ring-ref ring 0) + history)))) (when (= count ring-size) - (ring-extend ring (min (- comint-input-ring-size ring-size) - ring-size)) + (ring-extend ring (min (- ring-size-max ring-size) ring-size)) (setq ring-size (ring-size ring))) (ring-insert-at-beginning ring history) (setq count (1+ count)))))) --=_vtE9f0MKYXtD35YXUxzlpw4 Content-Type: text/x-patch; charset="us-ascii"; name*0="0002.comint_.don't.strip.anything.by.default.on.comint-input-ring-re"; name*1="ad.diff" Content-Disposition: attachment; filename*0= "0002.comint_.don't.strip.anything.by.default.on.comint-input-rin"; filename*1="g-read.diff"; size=715 #------------------------------------------------------------ #revno: 112737 #committer: Pete Beardmore #branch nick: bzr #timestamp: Thu 2013-05-30 13:20:40 +0100 #message: # comint: don't strip anything by default on comint-input-ring-read === modified file 'lisp/comint.el' --- lisp/comint.el 2013-05-30 12:17:39 +0000 +++ lisp/comint.el 2013-05-30 12:20:40 +0000 @@ -318,7 +318,7 @@ (defvar comint-input-ring-separator "\n" "Separator between commands in the history file.") -(defvar comint-input-history-ignore "^#" +(defvar comint-input-history-ignore "" "Regexp for history entries that should be ignored when Comint initializes.") (defcustom comint-process-echoes nil --=_vtE9f0MKYXtD35YXUxzlpw4 Content-Type: text/x-patch; charset="us-ascii"; name="0003.comint_.don't.match.an.empty.ignore.string.diff" Content-Disposition: attachment; filename="0003.comint_.don't.match.an.empty.ignore.string.diff"; size=840 #------------------------------------------------------------ #revno: 112738 #committer: Pete Beardmore #branch nick: bzr #timestamp: Thu 2013-05-30 13:29:26 +0100 #message: # comint: don't match an empty ignore string === modified file 'lisp/comint.el' --- lisp/comint.el 2013-05-30 12:20:40 +0000 +++ lisp/comint.el 2013-05-30 12:29:26 +0000 @@ -958,7 +958,8 @@ (point-min))) (setq history (buffer-substring start end)) (goto-char start) - (when (and (not (string-match ignore history)) + (when (and (or (= (length ignore) 0) + (not (string-match ignore history))) (or (null ignoredups) (ring-empty-p ring) (not (string-equal (ring-ref ring 0) --=_vtE9f0MKYXtD35YXUxzlpw4 Content-Type: text/x-patch; charset="us-ascii"; name="0004.comint_.fix.ignore-dupe.functionality.diff" Content-Disposition: attachment; filename="0004.comint_.fix.ignore-dupe.functionality.diff"; size=841 #------------------------------------------------------------ #revno: 112739 #committer: Pete Beardmore #branch nick: bzr #timestamp: Thu 2013-05-30 13:31:52 +0100 #message: # comint: fix ignore-dupe functionality === modified file 'lisp/comint.el' --- lisp/comint.el 2013-05-30 12:29:26 +0000 +++ lisp/comint.el 2013-05-30 12:31:52 +0000 @@ -962,7 +962,7 @@ (not (string-match ignore history))) (or (null ignoredups) (ring-empty-p ring) - (not (string-equal (ring-ref ring 0) + (not (string-equal (ring-ref ring (1- count)) history)))) (when (= count ring-size) (ring-extend ring (min (- ring-size-max ring-size) ring-size)) --=_vtE9f0MKYXtD35YXUxzlpw4--