From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED!not-for-mail From: Karl Fogel Newsgroups: gmane.emacs.devel Subject: Re: Don't complain about changed file when it hasn't changed Date: Tue, 06 Sep 2016 16:41:25 -0500 Message-ID: <87oa40k8oq.fsf@red-bean.com> References: Reply-To: Karl Fogel NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: text/plain X-Trace: blaine.gmane.org 1473198369 27251 195.159.176.226 (6 Sep 2016 21:46:09 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Tue, 6 Sep 2016 21:46:09 +0000 (UTC) User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.1.50 (gnu/linux) Cc: emacs-devel@gnu.org To: Stefan Monnier Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Tue Sep 06 23:46:04 2016 Return-path: Envelope-to: ged-emacs-devel@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 1bhOBq-00061L-F3 for ged-emacs-devel@m.gmane.org; Tue, 06 Sep 2016 23:45:58 +0200 Original-Received: from localhost ([::1]:36148 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bhOBo-0001XQ-2Y for ged-emacs-devel@m.gmane.org; Tue, 06 Sep 2016 17:45:56 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:43629) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bhO7Z-0003ny-BW for emacs-devel@gnu.org; Tue, 06 Sep 2016 17:41:34 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bhO7V-00010o-9N for emacs-devel@gnu.org; Tue, 06 Sep 2016 17:41:32 -0400 Original-Received: from mail-oi0-x242.google.com ([2607:f8b0:4003:c06::242]:33599) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bhO7V-00010Y-2r for emacs-devel@gnu.org; Tue, 06 Sep 2016 17:41:29 -0400 Original-Received: by mail-oi0-x242.google.com with SMTP id w78so11622417oie.0 for ; Tue, 06 Sep 2016 14:41:28 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=sender:from:to:cc:subject:references:reply-to:date:in-reply-to :message-id:user-agent:mime-version; bh=NkEKGvLU1bbpNFE2XYS19Q6VGqph6lcnJpzhKFTs6Fw=; b=wQ/Xxfrm3IP4ZyHLKE0Btew53rwSoDR4VxfFua0JnLRBrhQET9OcZ4OLl92MP6TG53 ZeYWDL9oBANJfuRRGCF2bIoh6P8L2aJYQ1XudodUdGBIaAem3a3qocY9sJRodtrQPC1U kBRi6HjOrmPO1OK349RzjzPy5cnyvpPfYQbUb8thK4UjfjXzAl5/1LFT/KlRPZe9Dx66 JrNotPNkkBTJoGfxlj6JY6Be6UuC+/sqSiWliBBkhB9qsmnpAeg00zrZoh5/Ne2UeG79 vArCFDyWy3hiSxK0WeOv20cMmjqPn8cY+dGjeZR8BgiQYENX5uEQ9R1wknEYXFMmpVls mb3w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:sender:from:to:cc:subject:references:reply-to :date:in-reply-to:message-id:user-agent:mime-version; bh=NkEKGvLU1bbpNFE2XYS19Q6VGqph6lcnJpzhKFTs6Fw=; b=MqiWMkNnCzfgCRheUGxxK10vtt2bLzCfTEa14g9S03WQoFBYO0S8Yna/EtSSNrKlBT mrC4eyUqpCXTFG+dp6ONOFW+SE6z2l0Y/FbtOLVIm+3jorB8BG54nhPwALLnmaWlvfZb gtF891E9zNwvrw3031ZsRhtjyO6Wswz6jAD/JQ3T8mPBI6nDFO0XCOHxaCf5hcSkGAaf +2Cjvt8gkiVEBLOj65vs1y9r8CghX76TxME+KR+bfxeJFKo+QCX8h21CeeMm0PhC9hSH jB/R9EJa9MfEE8QkRyDBb0EY7hmQdcS+DJGLeYpPBa3Da8OZpCZ+J7ooDaz4j9EiHZAA 2NPQ== X-Gm-Message-State: AE9vXwOQcMvfqHT90JfiqIwnU+VgSqRSfoiZpzliHZYUG7OzysDVAgRn6J6ullUYiax0PA== X-Received: by 10.36.74.138 with SMTP id k132mr1314003itb.85.1473198088324; Tue, 06 Sep 2016 14:41:28 -0700 (PDT) Original-Received: from kwork (74-92-190-114-Illinois.hfc.comcastbusiness.net. [74.92.190.114]) by smtp.gmail.com with ESMTPSA id f9sm575309itc.10.2016.09.06.14.41.26 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Tue, 06 Sep 2016 14:41:26 -0700 (PDT) In-Reply-To: (Stefan Monnier's message of "Tue, 06 Sep 2016 13:50:09 -0400") X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 2607:f8b0:4003:c06::242 X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Original-Sender: "Emacs-devel" Xref: news.gmane.org gmane.emacs.devel:207223 Archived-At: Stefan Monnier writes: >> Stefan, what is the performance impact of this change, and how often >> will that cost be paid? > >If you look at the code, you'll see that it is only called when the >previous behavior would have been to prompt the user. > >I'm sure you've already bumped into those prompts, but I'd be really >surprised if you've bumped into them frequently enough to be worried >about their impact on battery life. Stefan, I noticed you put a "FIXME" comment in the new code, about how it would be better to check the file size first. That would obviously be a good optimization: most of the time -- like, 99% of the time -- if the file contents are different from the buffer contents, their sizes will differ as well, so it's an easy "early out" check that would allow us to avoid loading in the entire file contents in the vast majority of cases. Since it's also fairly easy to do, I thought maybe there's some subtle reason you didn't do it. In an earlier comment, you mention encrypted files (and of course perhaps compressed files are another case), where the file size on disk might differ from the buffer size even though the contents are actually "the same". If those kinds of things were the reason you didn't code the early-out check, then do we have a canonical list anywhere of all possible "non-verbatim" buffer<->file relationships? E.g., compressed, encrypted... what else? It would be a shame not to have the optimization that would work fine in the majority of cases, but of course we need to make absolutely sure it's correct for all cases. I'm not sure how difficult that would be, but thought maybe you'd considered that question, and I'd like to know how deep this rabbit hole goes. (If it's not that deep, I'll address the FIXME.) Your patch is below, for convenient reference. Best regards, -Karl ============================================= git log --name-status 5a4bffb661^..5a4bffb661 ============================================= commit 5a4bffb6617a274ca19bc7f5c1b1ceb6345651ab Author: Stefan Monnier Date: Fri Sep 2 11:44:13 2016 -0400 Check actual contents before promting about changed file * lisp/userlock.el (userlock--check-content-unchanged) (userlock--ask-user-about-supersession-threat): New functions. * src/filelock.c (lock_file): Use them to avoid spurious prompting. * doc/lispref/buffers.texi (Modification Time): Update doc of ask-user-about-supersession-threat. M doc/lispref/buffers.texi M etc/NEWS M lisp/userlock.el M src/filelock.c ================================ git diff 5a4bffb661^..5a4bffb661 ================================ diff --git doc/lispref/buffers.texi doc/lispref/buffers.texi index 740d7cf..e4ef4d5 100644 --- doc/lispref/buffers.texi +++ doc/lispref/buffers.texi @@ -669,8 +669,9 @@ Modification Time This function is used to ask a user how to proceed after an attempt to modify an buffer visiting file @var{filename} when the file is newer than the buffer text. Emacs detects this because the modification -time of the file on disk is newer than the last save-time of the -buffer. This means some other program has probably altered the file. +time of the file on disk is newer than the last save-time and its contents +have changed. +This means some other program has probably altered the file. @kindex file-supersession Depending on the user's answer, the function may return normally, in diff --git etc/NEWS etc/NEWS index 18975cb..3092e9f 100644 --- etc/NEWS +++ etc/NEWS @@ -213,6 +213,10 @@ In modes where form feed was treated as a whitespace character, It now deletes whitespace after the last form feed thus behaving the same as in modes where the character is not whitespace. +** No more prompt about changed file when the file's content is unchanged. +Instead of only checking the modification time, Emacs now also checks +the file's actual content before prompting the user. + * Changes in Specialized Modes and Packages in Emacs 25.2 diff --git lisp/userlock.el lisp/userlock.el index a0c55fd..1cfc3b9 100644 --- lisp/userlock.el +++ lisp/userlock.el @@ -97,6 +97,41 @@ ask-user-about-lock-help (define-error 'file-supersession nil 'file-error) +(defun userlock--check-content-unchanged (fn) + (with-demoted-errors "Unchanged content check: %S" + ;; Even tho we receive `fn', we know that `fn' refers to the current + ;; buffer's file. + (cl-assert (equal fn (expand-file-name buffer-file-truename))) + ;; Note: rather than read the file and compare to the buffer, we could save + ;; the buffer and compare to the file, but for encrypted data this + ;; wouldn't work well (and would risk exposing the data). + (save-restriction + (widen) + (let ((buf (current-buffer)) + (cs buffer-file-coding-system) + (start (point-min)) + (end (point-max))) + ;; FIXME: To avoid a slow `insert-file-contents' on large or + ;; remote files, it'd be good to include file size in the + ;; "visited-modtime" check. + (when (with-temp-buffer + (let ((coding-system-for-read cs) + (non-essential t)) + (insert-file-contents fn)) + (when (= (buffer-size) (- end start)) ;Minor optimization. + (= 0 (let ((case-fold-search nil)) + (compare-buffer-substrings + buf start end + (current-buffer) (point-min) (point-max)))))) + (set-visited-file-modtime) + 'unchanged))))) + +;;;###autoload +(defun userlock--ask-user-about-supersession-threat (fn) + ;; Called from filelock.c. + (unless (userlock--check-content-unchanged fn) + (ask-user-about-supersession-threat fn))) + ;;;###autoload (defun ask-user-about-supersession-threat (fn) "Ask a user who is about to modify an obsolete buffer what to do. diff --git src/filelock.c src/filelock.c index 2f92e0f..bde34e2 100644 --- src/filelock.c +++ src/filelock.c @@ -684,7 +684,7 @@ lock_file (Lisp_Object fn) if (!NILP (subject_buf) && NILP (Fverify_visited_file_modtime (subject_buf)) && !NILP (Ffile_exists_p (fn))) - call1 (intern ("ask-user-about-supersession-threat"), fn); + call1 (intern ("userlock--ask-user-about-supersession-threat"), fn); }