bug#48535: 28.0.50; ff-find-other-file no longer sets current-buffer?

bug#48535: 28.0.50; ff-find-other-file no longer sets current-buffer?

[Stephen Leake]

[-- Attachment #1: Type: text/plain, Size: 3637 bytes --]

There is apparently some change in master relative to 27 in the way
ff-find-other-file works.

First the data from report-emacs-bug:

In GNU Emacs 28.0.50 (build 1, x86_64-w64-mingw32)
 of 2021-05-19 built on TAKVER4
Repository revision: 68f5718c0a4393391ce3c4179e8ab1430e5b0119
Repository branch: master
Windowing system distributor 'Microsoft Corp.', version 6.3.9600
System Description: Microsoft Windows 8.1 Pro (v6.3.0.9600.20018)

Configured using:
 'configure
 PKG_CONFIG_PATH=/mingw64/lib/pkgconfig:/mingw64/share/pkgconfig'

Configured features:
ACL DBUS GIF GMP GNUTLS HARFBUZZ JPEG JSON LCMS2 LIBXML2 MODULES NOTIFY
W32NOTIFY PDUMPER PNG RSVG SOUND THREADS TIFF TOOLKIT_SCROLL_BARS XPM
ZLIB

Important settings:
  value of $LC_CTYPE: en_US.UTF-8
  value of $LANG: ENU
  locale-coding-system: cp1252

Major mode: C/*l

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t
  abbrev-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message rmc puny dired dired-loaddefs
rfc822 mml mml-sec epa derived epg epg-config gnus-util rmail
rmail-loaddefs auth-source cl-seq eieio eieio-core cl-macs
eieio-loaddefs password-cache json map text-property-search time-date
subr-x seq byte-opt gv bytecomp byte-compile cconv mm-decode mm-bodies
mm-encode mail-parse rfc2231 mailabbrev gmm-utils mailheader sendmail
rfc2047 rfc2045 ietf-drums mm-util mail-prsvr mail-utils find-file
cc-mode cc-fonts cc-guess cc-menus cc-cmds cc-styles cc-align cc-engine
cc-vars cc-defs cl-loaddefs cl-lib iso-transl tooltip eldoc electric
uniquify ediff-hook vc-hooks lisp-float-type mwheel dos-w32 ls-lisp
disp-table term/w32-win w32-win w32-vars term/common-win tool-bar dnd
fontset image regexp-opt fringe tabulated-list replace newcomment
text-mode elisp-mode lisp-mode prog-mode register page tab-bar menu-bar
rfn-eshadow isearch easymenu timer select scroll-bar mouse jit-lock
font-lock syntax font-core term/tty-colors frame minibuffer cl-generic
cham georgian utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao
korean japanese eucjp-ms cp51932 hebrew greek romanian slovak czech
european ethiopic indian cyrillic chinese composite charscript charprop
case-table epa-hook jka-cmpr-hook help simple abbrev obarray
cl-preloaded nadvice button loaddefs faces cus-face macroexp files
window text-properties overlay sha1 md5 base64 format env code-pages
mule custom widget hashtable-print-readable backquote threads w32notify
dbusbind w32 lcms2 multi-tty make-network-process emacs)

Memory information:
((conses 16 74450 8788)
 (symbols 48 8432 1)
 (strings 32 25401 1600)
 (string-bytes 1 930349)
 (vectors 16 15287)
 (vector-slots 8 197771 11492)
 (floats 8 29 48)
 (intervals 56 237 0)
 (buffers 992 12))


To reproduce:

Copy the two attached files into one directory. Visit a_file.h, position point
after the last line (an elisp sexp in a C comment), type C-x C-e. A new
window opens visiting a_file.c, that buffer is current, as expected.

Now look at the message displayed; it is the result of (current-buffer).
In emacs 27, it is #<buffer a_file.c>, as expected. But in emacs master,
it is #<buffer a_file.h>.

Apparently (ff-find-other-file) returns to the current buffer after
opening the other window, even though the other buffer is current after
the progn finishes?

--
-- Stephe

[-- Attachment #2: a_file.h --]
[-- Type: application/octet-stream, Size: 86 bytes --]

// just a file with a .h extension

// (progn (ff-find-other-file t)(current-buffer))

[-- Attachment #3: a_file.c --]
[-- Type: application/octet-stream, Size: 85 bytes --]

// just a file with a .c extension
// (progn (ff-find-other-file t)(current-buffer))

bug#48535: 28.0.50; ff-find-other-file no longer sets current-buffer?

[Stephen Leake]

This breaks emacs ada-mode automated tests, which rely on evaluating
elisp forms in Ada comments.

git bisect says this commit is to blame:

commit: c105017c44d4a679f7af739b2c0390b2c7850569
Author: Stefan Monnier <monnier@iro.umontreal.ca>
Date:   Tue Apr 6 17:06:07 2021 -0400

    * lisp/find-file.el: Make the commands oblivious to mouse/non-mouse

    (ff-find-other-file): Add `event` argument.
    (ff-find-other-file-other-window): Rename from
    `ff-mouse-find-other-file-other-window` and use this new argument.
    (ff-mouse-find-other-file, ff-mouse-find-other-file-other-window):
    Make them obsolete aliases.
    (ff-upcase-p): Remove unused `start` and `end` arguments and
    simplify accordingly.

I looked thru the diff, but I didn't see anything obvious, so I'm
blaming the lexical-binding change. But I don't know how to fix it.

I've confirmed that disabling lexical-binding fixes the problem.

-- 
-- Stephe

bug#48535: 28.0.50; ff-find-other-file no longer sets current-buffer?

[Stephen Leake]

> I've confirmed that disabling lexical-binding fixes the problem.

On the other hand, re-enabling lexical-binding, and byte-compiling just
that file (which 'make' does), the problem does not come back. Sigh.

-- 
-- Stephe

bug#48535: 28.0.50; ff-find-other-file no longer sets current-buffer?

[Stephen Leake]

Stephen Leake <stephen_leake@stephe-leake.org> writes:

>> I've confirmed that disabling lexical-binding fixes the problem.
>
> On the other hand, re-enabling lexical-binding, and byte-compiling just
> that file (which 'make' does), the problem does not come back. Sigh.

I was confused; I was working in a 'good' commit, not the first 'bad'
commit.

Disabling lexical-binding in find-file.el does not fix the problem in
the 'bad' commit.
-- 
-- Stephe

bug#48535: 28.0.50; ff-find-other-file no longer sets current-buffer?

[Stephen Leake]

Stephen Leake <stephen_leake@stephe-leake.org> writes:

> Stephen Leake <stephen_leake@stephe-leake.org> writes:
>
>>> I've confirmed that disabling lexical-binding fixes the problem.
>>
>> On the other hand, re-enabling lexical-binding, and byte-compiling just
>> that file (which 'make' does), the problem does not come back. Sigh.
>
> I was confused; I was working in a 'good' commit, not the first 'bad'
> commit.
>
> Disabling lexical-binding in find-file.el does not fix the problem in
> the 'bad' commit.

The attached patch fixes the problem, in master HEAD (c9655fcb47).

-- 
-- Stephe

bug#48535: 28.0.50; ff-find-other-file no longer sets current-buffer?

[Stephen Leake]

[-- Attachment #1: Type: text/plain, Size: 695 bytes --]

Stephen Leake <stephen_leake@stephe-leake.org> writes:

> Stephen Leake <stephen_leake@stephe-leake.org> writes:
>
>> Stephen Leake <stephen_leake@stephe-leake.org> writes:
>>
>>>> I've confirmed that disabling lexical-binding fixes the problem.
>>>
>>> On the other hand, re-enabling lexical-binding, and byte-compiling just
>>> that file (which 'make' does), the problem does not come back. Sigh.
>>
>> I was confused; I was working in a 'good' commit, not the first 'bad'
>> commit.
>>
>> Disabling lexical-binding in find-file.el does not fix the problem in
>> the 'bad' commit.
>
> The attached patch fixes the problem, in master HEAD (c9655fcb47).

actually attach the patch
-- 
-- Stephe

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: find-file.diff --]
[-- Type: text/x-patch, Size: 1404 bytes --]

diff --git a/lisp/find-file.el b/lisp/find-file.el
index 6c3c0f123b..4fd4f4e06b 100644
--- a/lisp/find-file.el
+++ b/lisp/find-file.el
@@ -305,6 +305,10 @@ ff-find-other-file
 If optional IN-OTHER-WINDOW is non-nil, find the file in the other window.
 If optional IGNORE-INCLUDE is non-nil, ignore being on `#include' lines.
 
+If optional EVENT is non-nil (default `last-nonmenu-event', move
+point to the end position of that event before calling the
+various ff-* hooks.
+
 Variables of interest include:
 
  - `ff-case-fold-search'
@@ -351,10 +355,16 @@ ff-find-other-file
  - `ff-file-created-hook'
    List of functions to be called if the other file has been created."
   (interactive (list current-prefix-arg nil last-nonmenu-event))
-  (save-excursion
+  ;; We want to preserve point in the current buffer. But the point of
+  ;; ff-find-the-other-file is to make the the other file buffer
+  ;; current, so we can't use save-excursion here (see bug 48535).
+  (let ((start-buffer (current-buffer))
+        (start-point (point)))
     (posn-set-point (event-end event))
     (let ((ff-ignore-include ignore-include))
-      (ff-find-the-other-file in-other-window))))
+      (ff-find-the-other-file in-other-window))
+    (with-current-buffer start-buffer
+      (goto-char start-point))))
 
 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
 ;; Support functions

bug#48535: 28.0.50; ff-find-other-file no longer sets current-buffer?

[Lars Ingebrigtsen]

Stephen Leake <stephen_leake@stephe-leake.org> writes:

>> The attached patch fixes the problem, in master HEAD (c9655fcb47).
>
> actually attach the patch

Thanks; applied to Emacs 28.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no