From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Matt Armstrong Newsgroups: gmane.emacs.bugs Subject: bug#46701: [PATCH] small cleanups related to `unlock-buffer' Date: Mon, 22 Feb 2021 16:56:44 -0800 Message-ID: References: Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="31881"; mail-complaints-to="usenet@ciao.gmane.io" Cc: 46701@debbugs.gnu.org To: Eli Zaretskii Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Tue Feb 23 01:57:19 2021 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1lEM0t-00089r-Ll for geb-bug-gnu-emacs@m.gmane-mx.org; Tue, 23 Feb 2021 01:57:19 +0100 Original-Received: from localhost ([::1]:38860 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1lEM0s-0001Kd-HM for geb-bug-gnu-emacs@m.gmane-mx.org; Mon, 22 Feb 2021 19:57:18 -0500 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:40650) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lEM0c-0001KS-9G for bug-gnu-emacs@gnu.org; Mon, 22 Feb 2021 19:57:02 -0500 Original-Received: from debbugs.gnu.org ([209.51.188.43]:46388) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1lEM0c-0004a2-2P for bug-gnu-emacs@gnu.org; Mon, 22 Feb 2021 19:57:02 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1lEM0c-0007xj-1X for bug-gnu-emacs@gnu.org; Mon, 22 Feb 2021 19:57:02 -0500 X-Loop: help-debbugs@gnu.org Resent-From: Matt Armstrong Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Tue, 23 Feb 2021 00:57:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 46701 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch Original-Received: via spool by 46701-submit@debbugs.gnu.org id=B46701.161404181930598 (code B ref 46701); Tue, 23 Feb 2021 00:57:02 +0000 Original-Received: (at 46701) by debbugs.gnu.org; 23 Feb 2021 00:56:59 +0000 Original-Received: from localhost ([127.0.0.1]:57934 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lEM0Y-0007xR-ND for submit@debbugs.gnu.org; Mon, 22 Feb 2021 19:56:59 -0500 Original-Received: from relay11.mail.gandi.net ([217.70.178.231]:51181) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lEM0W-0007xE-NB for 46701@debbugs.gnu.org; Mon, 22 Feb 2021 19:56:57 -0500 Original-Received: from matts-mbp-2016.lan (24-113-169-116.wavecable.com [24.113.169.116]) (Authenticated sender: matt@rfc20.org) by relay11.mail.gandi.net (Postfix) with ESMTPSA id 6FA19100004; Tue, 23 Feb 2021 00:56:47 +0000 (UTC) In-Reply-To: <831rd8vzim.fsf@gnu.org> X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list 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-mx.org@gnu.org Original-Sender: "bug-gnu-emacs" Xref: news.gmane.io gmane.emacs.bugs:200636 Archived-At: --=-=-= Content-Type: text/plain Eli Zaretskii writes: >> From: Matt Armstrong >> Date: Sun, 21 Feb 2021 20:18:44 -0800 >> >> Subject: [PATCH 1/4] Remove unecessary (unlock-buffer) calls. >> >> * lisp/type-break.el (type-break-mode): Remove an (unlock-buffer) call >> implied by (set-buffer-modified nil). >> * lisp/simple.el (primitive-undo): ditto. > > My reading of the code is that the above is true only if > inhibit-modification-hooks is nil. Otherwise, these calls are not > no-ops. Thanks, good catch. I think the change to type-break.el is probably fine, but I will drop it for now. In simple.el I just removed the unecessary fboundp check (new patch attached). >> --- a/lisp/files.el >> +++ b/lisp/files.el >> @@ -6234,11 +6234,8 @@ revert-buffer-insert-file-contents--default-function >> "Cannot revert unreadable file %s") >> file-name)) >> (t >> - ;; Bind buffer-file-name to nil >> - ;; so that we don't try to lock the file. >> - (let ((buffer-file-name nil)) >> - (or auto-save-p >> - (unlock-buffer))) >> + (unless auto-save-p >> + (unlock-buffer)) > > And here, I think we just forgot to update the Lisp code when > unlock-buffer started to look at buffer-file-truename instead of > buffer-file-name. But otherwise, I see no reason why we should remove > the call to unlock-buffer; what did I miss? This is some very old code. When originally written the `unlock-buffer' was a nop because it was well before file-buffer-truename existed -- the let bind disabled it. The let bind was intended to prevent locking the buffer by `erase-buffer'. See Roland's commit below, which is the first commit for this file that we've got. b4da00e92a0 (Roland McGrath 1991-07-19 1804) ;; Bind buffer-file-name to nil ;; so that we don't try to lock the file. (let ((buffer-file-name nil)) (or auto-save-p (unlock-buffer)) (erase-buffer)) (insert-file-contents file-name (not auto-save-p)))) A few years later Richard moved the erase logic to insert-file-contents, but left the no-op call to unlock-buffer. Note that the comment no longer makes sense. f9456b0a5b5 (Richard M. Stallman 1994-02-17 2020) ;; Bind buffer-file-name to nil ;; so that we don't try to lock the file. (let ((buffer-file-name nil)) (or auto-save-p (unlock-buffer)) (insert-file-contents file-name (not auto-save-p) nil nil t))) insert-file-contents has logic to unlock the buffer. I must admit that I don't understand how that logic works. When unlock-buffer switched to using buffer-file-tempfile the unlock-buffer call here became active for the first time, probably by accident? Removing it now is a possible behavior change, but it restores the original behavior. I did a manual test. I edited a file, then changed the file outside emacs, then ran revert-buffer. The file's lock file was still removed, even with the patch below applied. --=-=-= Content-Type: text/x-patch Content-Disposition: inline; filename=0001-Remove-unecessary-unlock-buffer-call.patch >From 3b569a6b9139d2b350745bebc64db506728cf994 Mon Sep 17 00:00:00 2001 From: Matt Armstrong Date: Mon, 22 Feb 2021 16:40:05 -0800 Subject: [PATCH 1/2] Remove unecessary unlock-buffer call. * lisp/files.el (revert-buffer-insert-file-contents--default-function): Remove vestigial call to `unlock-buffer'. --- lisp/files.el | 5 ----- 1 file changed, 5 deletions(-) diff --git a/lisp/files.el b/lisp/files.el index 68e883513c..b5d92cd841 100644 --- a/lisp/files.el +++ b/lisp/files.el @@ -6234,11 +6234,6 @@ revert-buffer-insert-file-contents--default-function "Cannot revert unreadable file %s") file-name)) (t - ;; Bind buffer-file-name to nil - ;; so that we don't try to lock the file. - (let ((buffer-file-name nil)) - (or auto-save-p - (unlock-buffer))) (widen) (let ((coding-system-for-read ;; Auto-saved file should be read by Emacs's -- 2.30.0 --=-=-= Content-Type: text/x-patch Content-Disposition: inline; filename=0002-Assume-unlock-buffer-is-always-bound.patch >From dc9cc451e0ab6a84d839f3ed9d1b552da3c43373 Mon Sep 17 00:00:00 2001 From: Matt Armstrong Date: Mon, 22 Feb 2021 16:41:44 -0800 Subject: [PATCH 2/2] Assume unlock-buffer is always bound. * lisp/simple.el (primitive-undo): Assume unlock-buffer is always bound. --- lisp/simple.el | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lisp/simple.el b/lisp/simple.el index 1dfc3374ad..c4062d97cc 100644 --- a/lisp/simple.el +++ b/lisp/simple.el @@ -3043,8 +3043,7 @@ primitive-undo (and (consp time) (equal (list (car time) (cdr time)) (visited-file-modtime)))) - (when (fboundp 'unlock-buffer) - (unlock-buffer)) + (unlock-buffer) (set-buffer-modified-p nil))) ;; Element (nil PROP VAL BEG . END) is property change. (`(nil . ,(or `(,prop ,val ,beg . ,end) pcase--dontcare)) -- 2.30.0 --=-=-=--