From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Stefan Monnier Newsgroups: gmane.emacs.bugs Subject: bug#23871: 25.1.50; Undo unexpectedly leads to blank buffer Date: Sun, 03 Jul 2016 17:33:15 -0400 Message-ID: References: <83h9cavdgj.fsf@gnu.org> <87poqyy2tc.fsf@metalevel.at> <87vb0qqrkz.fsf@russet.org.uk> <87h9c9zx75.fsf@metalevel.at> <834m89vmyv.fsf@gnu.org> <878txlsbdb.fsf@russet.org.uk> <87furtccdv.fsf@metalevel.at> <877fd5q9te.fsf@russet.org.uk> <83bn2gtruk.fsf@gnu.org> <87k2h37pvb.fsf@russet.org.uk> NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: text/plain X-Trace: ger.gmane.org 1467581675 4204 80.91.229.3 (3 Jul 2016 21:34:35 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Sun, 3 Jul 2016 21:34:35 +0000 (UTC) Cc: 23871@debbugs.gnu.org, triska@metalevel.at To: phillip.lord@russet.org.uk (Phillip Lord) Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Sun Jul 03 23:34:21 2016 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 1bJp1w-0002mZ-BG for geb-bug-gnu-emacs@m.gmane.org; Sun, 03 Jul 2016 23:34:20 +0200 Original-Received: from localhost ([::1]:44195 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bJp1v-0006zz-KZ for geb-bug-gnu-emacs@m.gmane.org; Sun, 03 Jul 2016 17:34:19 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:58754) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bJp1i-0006v0-G5 for bug-gnu-emacs@gnu.org; Sun, 03 Jul 2016 17:34:07 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bJp1e-0004JQ-C6 for bug-gnu-emacs@gnu.org; Sun, 03 Jul 2016 17:34:06 -0400 Original-Received: from debbugs.gnu.org ([208.118.235.43]:52173) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bJp1e-0004JL-8u for bug-gnu-emacs@gnu.org; Sun, 03 Jul 2016 17:34:02 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1bJp1e-0006u6-54 for bug-gnu-emacs@gnu.org; Sun, 03 Jul 2016 17:34:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Stefan Monnier Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Sun, 03 Jul 2016 21:34:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 23871 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: Original-Received: via spool by 23871-submit@debbugs.gnu.org id=B23871.146758160426495 (code B ref 23871); Sun, 03 Jul 2016 21:34:02 +0000 Original-Received: (at 23871) by debbugs.gnu.org; 3 Jul 2016 21:33:24 +0000 Original-Received: from localhost ([127.0.0.1]:36277 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1bJp11-0006tH-R3 for submit@debbugs.gnu.org; Sun, 03 Jul 2016 17:33:24 -0400 Original-Received: from smtp-as-01-06.vtxnet.net ([194.38.175.135]:49089 helo=smtp-as-01.vtxnet.net) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1bJp0x-0006t7-OM for 23871@debbugs.gnu.org; Sun, 03 Jul 2016 17:33:22 -0400 Original-Received: from smtp-as-01.vtxnet.net (smtp-as-01-04.vtxnet.net [194.38.175.133]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by smtp-as-01.vtxnet.net (Postfix) with ESMTPS id 0F94647F3C; Sun, 3 Jul 2016 23:33:19 +0200 (CEST) Original-Received: from smtp-pri-01.vtxnet.net (smtp-pri-01.vtxnet.net [212.147.62.135]) by smtp-as-01.vtxnet.net (Postfix) with ESMTP id 07F4F47F31; Sun, 3 Jul 2016 23:33:17 +0200 (CEST) Original-Received: from fmsmemgm.homelinux.net (dyn.144-85-234-142.dsl.vtx.ch [144.85.234.142]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by smtp-pri-01.vtxnet.net (VTX Services SA) with ESMTP id E9228CFCEC; Sun, 3 Jul 2016 23:33:16 +0200 (CEST) Original-Received: by fmsmemgm.homelinux.net (Postfix, from userid 20848) id F084EAE247; Sun, 3 Jul 2016 17:33:15 -0400 (EDT) In-Reply-To: <87k2h37pvb.fsf@russet.org.uk> (Phillip Lord's message of "Sat, 02 Jul 2016 21:21:28 +0100") User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.1.50 (gnu/linux) 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:120372 Archived-At: >> From d4e9e44402fdf248ba4bc895e914d4cc5580f229 Mon Sep 17 00:00:00 2001 > From: Phillip Lord > Date: Thu, 30 Jun 2016 22:06:00 +0100 > Subject: [PATCH] Fix missing point information in undo > * src/undo.c (record_insert): Use record_point instead of > prepare_record, and do so unconditionally. > (prepare_record): Do not record first change. > (record_point): Now conditional on state before the last command. > (record_delete): Call record_point unconditionally. I haven't had time to look in detail at this patch, so it's hard for me to give a strong agreement. Maybe some explanations would help. Could you give a verbosish "commit log" explaining the reason behind each hunk? > @@ -40,16 +40,13 @@ prepare_record (void) > /* Allocate a cons cell to be the undo boundary after this command. */ > if (NILP (pending_boundary)) > pending_boundary = Fcons (Qnil, Qnil); > - > - if (MODIFF <= SAVE_MODIFF) > - record_first_change (); > } Not sure why/how this is related to the other changes, nor why this code was there in the first place. But in any case, the comment before prepare_record needs to be updated, because it seems to describe neither the current behavior, nor the behavior after applying the above hunk. BTW, I notice that in the current code (emacs-25), there's one other call to record_first_change (in record_property_change), and it could be replaced with a call to prepare_record, so it begs the question whether the above hunk should also apply to record_property_change as well. > /* Record point as it was at beginning of this command. > - PT is the position of point that will naturally occur as a result of the > + BEG is the position of point that will naturally occur as a result of the > undo record that will be added just after this command terminates. */ > static void > -record_point (ptrdiff_t pt) > +record_point (ptrdiff_t beg) > { > /* Don't record position of pt when undo_inhibit_record_point holds. */ > if (undo_inhibit_record_point) > @@ -60,13 +57,16 @@ record_point (ptrdiff_t pt) > at_boundary = ! CONSP (BVAR (current_buffer, undo_list)) > || NILP (XCAR (BVAR (current_buffer, undo_list))); You said: The problem was caused because of undo only records point after a boundary (or the first element). I'd changed things during slightly when I update undo.c so that the timestamp list got added before checking whether I was at a boundary, hence blocking addition of the point restoration information. so maybe the computation of at_boundary above should be tweaked to ignore timestamps? > /* If we are just after an undo boundary, and > point wasn't at start of deleted range, record where it was. */ > - if (at_boundary) > + if (at_boundary > + && point_before_last_command_or_undo != beg > + && buffer_before_last_command_or_undo == current_buffer ) > bset_undo_list (current_buffer, > - Fcons (make_number (pt), > + Fcons (make_number (point_before_last_command_or_undo), > BVAR (current_buffer, undo_list))); I like this part. > - if (point_before_last_command_or_undo != beg > - && buffer_before_last_command_or_undo == current_buffer) > - record_point (point_before_last_command_or_undo); > + prepare_record (); > + > + record_point (beg); And I like this part too. Stefan