From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: martin rudalics Newsgroups: gmane.emacs.devel Subject: Re: undo weirdness with insert-file-contents Date: Sun, 02 Mar 2008 13:44:46 +0100 Message-ID: <47CAA13E.3040909@gmx.at> References: <47C70CFC.30203@gmx.at> <47C88A51.7020205@gmx.at> NNTP-Posting-Host: lo.gmane.org Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="------------000700040800030209070406" X-Trace: ger.gmane.org 1204461969 25482 80.91.229.12 (2 Mar 2008 12:46:09 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Sun, 2 Mar 2008 12:46:09 +0000 (UTC) Cc: Glenn Morris , emacs-devel@gnu.org, Bill Wohler , Miles Bader To: Stefan Monnier Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Sun Mar 02 13:46:34 2008 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([199.232.76.165]) by lo.gmane.org with esmtp (Exim 4.50) id 1JVnaK-0004Kq-Uj for ged-emacs-devel@m.gmane.org; Sun, 02 Mar 2008 13:46:33 +0100 Original-Received: from localhost ([127.0.0.1] helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1JVnZn-0003m2-OV for ged-emacs-devel@m.gmane.org; Sun, 02 Mar 2008 07:46:00 -0500 Original-Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1JVnZH-0003e7-0G for emacs-devel@gnu.org; Sun, 02 Mar 2008 07:45:27 -0500 Original-Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1JVnZD-0003d5-Hy for emacs-devel@gnu.org; Sun, 02 Mar 2008 07:45:25 -0500 Original-Received: from [199.232.76.173] (helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1JVnZB-0003ce-77 for emacs-devel@gnu.org; Sun, 02 Mar 2008 07:45:22 -0500 Original-Received: from mail.gmx.net ([213.165.64.20]) by monty-python.gnu.org with smtp (Exim 4.60) (envelope-from ) id 1JVnZA-00054p-Ef for emacs-devel@gnu.org; Sun, 02 Mar 2008 07:45:21 -0500 Original-Received: (qmail invoked by alias); 02 Mar 2008 12:45:16 -0000 Original-Received: from N925P028.adsl.highway.telekom.at (EHLO [62.47.59.156]) [62.47.59.156] by mail.gmx.net (mp054) with SMTP; 02 Mar 2008 13:45:16 +0100 X-Authenticated: #14592706 X-Provags-ID: V01U2FsdGVkX1+zBY15682zAe1QsfwqkMio2EbGVZ8vj/mmHOUNBc 3lxSDiECkS39Fb User-Agent: Mozilla Thunderbird 1.0 (Windows/20041206) X-Accept-Language: de-DE, de, en-us, en In-Reply-To: X-Y-GMX-Trusted: 0 X-detected-kernel: by monty-python.gnu.org: Linux 2.6, seldom 2.4 (older, 4) X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Original-Sender: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.devel:91056 Archived-At: This is a multi-part message in MIME format. --------------000700040800030209070406 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit >>+ current_buffer->undo_list = >>+ Fcons (Qnil, current_buffer->undo_list); > > > We should use Fundo_boundary here. OK - tho' I'm not sure whether pending_boundary should be considered here. Note that I could remove that boundary and also combine the insertion with a previous one if that's desired. Personally, I'd prefer to undo two separate insertions from files separately. In this context note that record_insert combines two insertions only if they happen in a left-to-right way. IIUC this should be eventually changed when Emacs goes bi-directional. >>+ else if (inserted > 0 >>+ && (! EQ (original_undo_list, Qt)) >>+ && (NILP (visit)) && (NILP (replace))) >>+ /* Assure that the first insertion is counted as a change. */ >>+ { >>+ current_buffer->undo_list = Qnil; >>+ record_first_change (); >>+ } > > > You use `else' between the two, but it's not clear why the two should be > mutually exclusive. And please mention `visit' in the comment. It's hopefully a bit clearer now although I'm not sure whether I need the empty_undo_list check at all. BTW, what is the semantics of `insert-file-contents' with VISIT nil and REPLACE non-nil? > Also if the file is empty, is this going to mark the buffer as modified > even though nothing was changed? I'm afraid I don't understand the question - do you mean the case where inserted equals zero? I don't handle that. But maybe I'm missing something. --------------000700040800030209070406 Content-Type: text/plain; name="fileio.patch" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="fileio.patch" *** fileio.c.~1.602.~ Thu Feb 14 20:41:44 2008 --- fileio.c Sun Mar 2 12:03:00 2008 *************** *** 3730,3735 **** --- 3730,3736 ---- int read_quit = 0; Lisp_Object old_Vdeactivate_mark = Vdeactivate_mark; int we_locked_file = 0; + int empty_undo_list = NILP (current_buffer->undo_list); if (current_buffer->base_buffer && ! NILP (visit)) error ("Cannot do file visiting in an indirect buffer"); *************** *** 4593,4598 **** --- 4594,4622 ---- if (CODING_MAY_REQUIRE_DECODING (&coding) && (inserted > 0 || CODING_REQUIRE_FLUSHING (&coding))) { + if (CONSP (current_buffer->undo_list) + && ! NILP (XCAR (current_buffer->undo_list))) + /* If the undo list is non-empty and undo information is + recorded, insert an undo boundary unless we already have one. + This is needed to avoid that record_insert combines the + present insertion with an earlier one and consequently the + mechanism fails that compares the start position of the + insertion before vs that after the format-decode step. */ + Fundo_boundary (); + else if (empty_undo_list + && MODIFF <= SAVE_MODIFF + && NILP (visit) && NILP (replace)) + /* Else if the present insertion is the very first change of the + current buffer, undo information is recorded, and we are + neither visiting a file nor replacing buffer contents, make + sure we record the insertion as a first change in the + undo-list. This is needed in order to avoid that undoing the + insertion leaves the buffer's modified state set. */ + { + current_buffer->undo_list = Qnil; + record_first_change (); + } + move_gap_both (PT, PT_BYTE); GAP_SIZE += inserted; ZV_BYTE -= inserted; *************** *** 4604,4611 **** coding_system = CODING_ID_NAME (coding.id); } else if (inserted > 0) ! adjust_after_insert (PT, PT_BYTE, PT + inserted, PT_BYTE + inserted, ! inserted); /* Now INSERTED is measured in characters. */ --- 4628,4651 ---- coding_system = CODING_ID_NAME (coding.id); } else if (inserted > 0) ! { ! if (CONSP (current_buffer->undo_list) ! && ! NILP (XCAR (current_buffer->undo_list))) ! /* Insert an undo boundary unless there's one here. See ! explanation above. */ ! Fundo_boundary (); ! else if (empty_undo_list ! && MODIFF <= SAVE_MODIFF ! && NILP (visit) && NILP (replace)) ! /* Make sure the first insertion is counted as a change. See ! explanation above. */ ! { ! current_buffer->undo_list = Qnil; ! record_first_change (); ! } ! adjust_after_insert (PT, PT_BYTE, PT + inserted, PT_BYTE + inserted, ! inserted); ! } /* Now INSERTED is measured in characters. */ --------------000700040800030209070406--