From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED!not-for-mail From: Allen Li Newsgroups: gmane.emacs.bugs Subject: bug#29118: 25.2.50; Undoing shell flush output results in weird state Date: Mon, 13 Nov 2017 00:29:10 -0800 Message-ID: References: <878tfe53fm.fsf@users.sourceforge.net> <8760af2vup.fsf@users.sourceforge.net> NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" X-Trace: blaine.gmane.org 1510561820 4342 195.159.176.226 (13 Nov 2017 08:30:20 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Mon, 13 Nov 2017 08:30:20 +0000 (UTC) Cc: 29118@debbugs.gnu.org, Barry OReilly , Stefan To: Noam Postavsky Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Mon Nov 13 09:30:10 2017 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by blaine.gmane.org with esmtp (Exim 4.84_2) (envelope-from ) id 1eEA87-0000Yg-NJ for geb-bug-gnu-emacs@m.gmane.org; Mon, 13 Nov 2017 09:30:08 +0100 Original-Received: from localhost ([::1]:53137 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eEA8E-0006zw-C8 for geb-bug-gnu-emacs@m.gmane.org; Mon, 13 Nov 2017 03:30:14 -0500 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:51917) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eEA86-0006yK-Kh for bug-gnu-emacs@gnu.org; Mon, 13 Nov 2017 03:30:07 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1eEA83-000782-Hi for bug-gnu-emacs@gnu.org; Mon, 13 Nov 2017 03:30:06 -0500 Original-Received: from debbugs.gnu.org ([208.118.235.43]:57190) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1eEA83-00077S-Dq for bug-gnu-emacs@gnu.org; Mon, 13 Nov 2017 03:30:03 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1eEA82-0007iA-VO for bug-gnu-emacs@gnu.org; Mon, 13 Nov 2017 03:30:02 -0500 X-Loop: help-debbugs@gnu.org Resent-From: Allen Li Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Mon, 13 Nov 2017 08:30:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 29118 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: confirmed Original-Received: via spool by 29118-submit@debbugs.gnu.org id=B29118.151056175829562 (code B ref 29118); Mon, 13 Nov 2017 08:30:02 +0000 Original-Received: (at 29118) by debbugs.gnu.org; 13 Nov 2017 08:29:18 +0000 Original-Received: from localhost ([127.0.0.1]:37638 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1eEA7K-0007gi-41 for submit@debbugs.gnu.org; Mon, 13 Nov 2017 03:29:18 -0500 Original-Received: from mail-qk0-f171.google.com ([209.85.220.171]:43074) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1eEA7I-0007gT-Sk for 29118@debbugs.gnu.org; Mon, 13 Nov 2017 03:29:17 -0500 Original-Received: by mail-qk0-f171.google.com with SMTP id 78so18722787qkz.0 for <29118@debbugs.gnu.org>; Mon, 13 Nov 2017 00:29:16 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=TVH9iGJ1ibmuEhdu1YZsr25AO64+Wmv/fa3Bycch3hE=; b=PWn0s1dJ8RU2vLFgh6cJisg1ckqOg2sYsfOvPBSCMDycwz1Nng7jKKBpWUQra6k49p k9sbE3CnCBJxjXc+xEvuyh0Ww1SNfEv/y/wD01+bMjtc6ntkOe3aq0YHHl+tZWT7M/UH G4BVCnleXhUrU8kJWP2wfn1iZVpeRdyWhYiCpyKGCeH9FpIcKRS4bs/v+AyXMYs2sRQS PC50F6Dt4XcZB/Ns19AKSmJM3hc9lSMr2rIYiN2RCVAhE74ZyVKJ7bx/YE1FMbIPoemg 00JSHSugaksr0PIzNxA7sVimKNKbhvG2tftULI8iFmz6tnf3CIsTJ/z7LOnHXrDOAB4z nidQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=TVH9iGJ1ibmuEhdu1YZsr25AO64+Wmv/fa3Bycch3hE=; b=c4s9Lo1wAOTJOjM5me89nU0aumuEG+0cia6zzKcD1G0zsvBwuIggRW6JesAaZzrRnU CQlVStiN4iyAy3WgcMh927R8pXNZvHbOaB5M50IjPRqRhAxrdqvkTPsW6Yl+JSd+UoHH NrebhC5qbRT0ipn0iQ4VsdiO8/99KYJV+zFkaiuyJadJOFGVOj4QP5/6z3LH5Z3od7dg fNPPYOp3gbFetKm7f/1lXS2TC1VKzFdxdAjQB6Cr96MtfsaPH9esS6HYEM6LfZueuwF2 xzPMFNeIWPtsaoxz5gQEixjgzjISYkph57LGK6SNmu/wPrOoyA9km4nJZrxY7+4LtL3F U0/Q== X-Gm-Message-State: AJaThX7s0IPg+xEBLVpiEJq6ysTq6m0PV/rxD3dg3J19pSmHA7oVGW9f nNIysXMef5EWXgO0CglAmOcYdaG3zAicnTu0k04= X-Google-Smtp-Source: AGs4zMZYGRi6ZYhPmUmw2KQXnD2C5lbnodNPEVIzPTsGO39zO9UKd6XRQ6+D/IXz2OadcqFxYSWJqYLZXYPrl+w4XEQ= X-Received: by 10.55.77.214 with SMTP id a205mr12147343qkb.196.1510561751129; Mon, 13 Nov 2017 00:29:11 -0800 (PST) Original-Received: by 10.237.53.92 with HTTP; Mon, 13 Nov 2017 00:29:10 -0800 (PST) In-Reply-To: <8760af2vup.fsf@users.sourceforge.net> X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 208.118.235.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" Xref: news.gmane.org gmane.emacs.bugs:139824 Archived-At: On Sun, Nov 12, 2017 at 10:30 AM, Noam Postavsky wrote: > Hmm, as far as I can tell, in this case the text was inserted on the > "normal" side of the marker, but then the marker is moved afterwards. > So I'm not sure if such tracking would help. > > (defun comint-output-filter (process string) > ... > ;; insert-before-markers is a bad thing. XXX > ;; Luckily we don't have to use it any more, we use > ;; window-point-insertion-type instead. > (insert string) > > ;; Advance process-mark > (set-marker (process-mark process) (point)) > > > However, while looking at the undo code, I noticed that it checks for > valid marker adjustments by checking that the marker position is the > same as the POSITION of the deletion record. However, for a negative > POSITION (indicating text was at the end of the deleted text) this > comparison will always be false (markers can't have negative positions). I was mistaken, undo does track marker movement correctly; or mostly correctly, excluding this bug. > > (defun primitive-undo (n list) > ... > (`(,(and string (pred stringp)) . ,(and pos (pred integerp))) > ... > (and (eq (marker-buffer m) (current-buffer)) > (= pos m) ; <<<<<<<<<<<< Always nil for negative `pos' > (push marker-adj valid-marker-adjustments)))) > > I don't see why the position of point should affect the adjustment of > other markers. This seems to have been added by [1: 37ea8275f7] to fix > Bug#16818. Before that, if I understand correctly, all marker > adjustments were applied without checking location at all. And indeed, > this bug does not occur in Emacs 24.3. I agree with your analysis. The intent of the code is to make sure that the marker hasn't been moved from the original location before trying to restore it. In that case, comparing against the absolute value of pos makes sense, as we use the sign to carry extra information in band that is not relevant to the marker adjustment. > So I would suggest the following patch, which does seem to actually fix > this bug. It might break some other things though; I have not tested it > apart from the scenario in this bug. I'm adding the participants of > Bug#16818 to Cc, in the hope they might have some more insight. > > --- i/lisp/simple.el > +++ w/lisp/simple.el > @@ -2579,7 +2579,7 @@ primitive-undo > (let* ((marker-adj (pop list)) > (m (car marker-adj))) > (and (eq (marker-buffer m) (current-buffer)) > - (= pos m) > + (= (abs pos) m) > (push marker-adj valid-marker-adjustments)))) > ;; Insert string and adjust point > (if (< pos 0) > > [1: 37ea8275f7]: 2014-03-24 22:47:39 -0400 > Undo in region after markers in undo history relocated > https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=37ea8275f7faad1192ddaba9f4a0789580675e17 I have added this to my trusty personal monkeypatch library. I can confirm that it fixes the current bug. I can use this patch for a while and see if I run into any obvious issues. The patch seems quite straightforward and correct, but perhaps I am naive.