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: Mon, 03 Mar 2008 10:09:09 +0100 Message-ID: <47CBC035.7030006@gmx.at> References: <47C70CFC.30203@gmx.at> <47C88A51.7020205@gmx.at> <47CAA13E.3040909@gmx.at> <47CB2498.1030405@gmx.at> NNTP-Posting-Host: lo.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit X-Trace: ger.gmane.org 1204535924 3021 80.91.229.12 (3 Mar 2008 09:18:44 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Mon, 3 Mar 2008 09:18:44 +0000 (UTC) Cc: emacs-devel@gnu.org To: Stefan Monnier Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Mon Mar 03 10:19:08 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 1JW6p9-0003ZC-Td for ged-emacs-devel@m.gmane.org; Mon, 03 Mar 2008 10:19:08 +0100 Original-Received: from localhost ([127.0.0.1] helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1JW6oc-0005wm-Rf for ged-emacs-devel@m.gmane.org; Mon, 03 Mar 2008 04:18:34 -0500 Original-Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1JW6oY-0005tO-F7 for emacs-devel@gnu.org; Mon, 03 Mar 2008 04:18:30 -0500 Original-Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1JW6oT-0005i7-90 for emacs-devel@gnu.org; Mon, 03 Mar 2008 04:18:29 -0500 Original-Received: from [199.232.76.173] (helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1JW6oT-0005hu-4p for emacs-devel@gnu.org; Mon, 03 Mar 2008 04:18:25 -0500 Original-Received: from mail.gmx.net ([213.165.64.20]) by monty-python.gnu.org with smtp (Exim 4.60) (envelope-from ) id 1JW6oS-0000fi-L5 for emacs-devel@gnu.org; Mon, 03 Mar 2008 04:18:25 -0500 Original-Received: (qmail invoked by alias); 03 Mar 2008 09:11:42 -0000 Original-Received: from N778P022.adsl.highway.telekom.at (EHLO [62.47.41.54]) [62.47.41.54] by mail.gmx.net (mp027) with SMTP; 03 Mar 2008 10:11:42 +0100 X-Authenticated: #14592706 X-Provags-ID: V01U2FsdGVkX19AGEs9hcFAMHh0luGDZyZI5OrwvCYg2K4pZBAQb5 P6dBeCLrSIgUWe 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:91157 Archived-At: > Currently it's done at 3 places: record_property_change, record_point > (both of them are in undo.c) and modify_region in insdel.c. > And record_point is called from record_insert. So normally it all > happens "automatically". Maybe it doesn't happen because the if (MODIFF <= SAVE_MODIFF) check fails. Anyway, the behavior reported by Glenn in an earlier posting (and also observed by me) indicates it doesn't happen. >>The main reason for the "mess" is that `insert-file-contents' has to >>reconcile visiting a file and inserting the contents of some file into >>an existing buffer. In the first case the buffer should appear >>unmodified and the undo-list empty after the insertion. > > When visiting a file, insert-file-contents is called from some other > piece of elisp which is the one that should take care of setting up the > undo-list and the unmodified status. It shouldn't be > insert-file-contents's job. Currently `insert-file-contents' is the only routine knowing 'bout whether it was able to do something with the REPLACE argument. That's also why I asked whether VISIT nil/REPLACE non-nil DTRT. We always have the dichotomy that from the user's point of view text is replaced from BEG to END while in fact text was replaced only from REALBEG >= BEG to REALEND =< REND. IIUC, the REPLACE stuff was mainly written to make reverting smoother. Since buffer reverting throws the undo list away, optimizing handling of the latter was hardly a concern. Also note: We never care about text-properties here - hence strictly spoken the REPLACE stuff _is_ incorrect. Suppose I visit a file and assign some text a face property - the buffer is modified. Next I revert the buffer and it appears unmodified although the text property is still there. In fact the only change caused by reverting was to make the buffer appear unmodified and the undo list empty. I know, with font-locking there's hardly a way to tell whether a text property is something that should be considered a "buffer change" or just a "hidden buffer change" as Alan calls it. Anyway: When you store text properties on file the current behavior is inherently wrong :-( I always wondered whether the REPLACE thingy could have been implemented easier by comparing the region around `point' before and after the replacement. Many routines apply such heuristics (bookmarks, etags, diff related programs). >>A second reason stems from the various decoding steps. Ideally, >>decoding should run transparently and only the "final" insertion get >>recorded. For this purpose you have to temporarily switch off undo >>recording in the non-visiting case. Otherwise, undoing changes could >>reveal changes done by the decoding routine as can be observed with >>Emacs 22. Moreover, recording changes during decoding might be >>expensive. > > I understand this part, but it seems there's got to be a simpler way. > > BTW, rather than use an undo-boundary, we could simply store the old > undo-list in some local var, then set undo-list to nil, then do the > dance, ... dancing with undo enabled? > then nconc the new undo-list and the old undo-list. Setting the > undo-list to nil temporarily hides the previous undo-list, thus > preventing merging entries. But _all_ we really need are the beginning and end of the inserted text. We can get them easily with undo recording turned off during decoding - once the problems with VISIT and REPLACE have been resolved. > Or rather create a new insert-file-contents-1 which doesn't take any > `visit' argument. ... but a REPLACE argument, I suppose. See the example with BEG and REALBEG above: Undo from BEG or REALBEG? Don't change the modified state when the region is (apparently) identic with that on the file?