[BUG] org-id-find might change the major-mode of a buffer [9.8-pre (release_9.7.8-713-g62cbac @ /home/viz/lib/emacs/straight/build/org/)]

[BUG] org-id-find might change the major-mode of a buffer [9.8-pre (release_9.7.8-713-g62cbac @ /home/viz/lib/emacs/straight/build/org/)]

[Visuwesh]

Remember to cover the basics, that is, what you expected to happen and
what in fact did happen.  You don't know how to make a good report?  See

     https://orgmode.org/manual/Feedback.html#Feedback

Your bug report will be posted to the Org mailing list.
------------------------------------------------------------------------

Hello,

I have code in my init that calls org-id-find to get the entry ID refers
to.  When the ID is non-existent, the function unintentionally changes
the major-mode of the current buffer.
[ This almost caused me to lose a PDF file, but thankfully I could
  recover it because of Emacs backup.  Admittedly this happened because
  I was half-panicking and half-confused.  ]

To reproduce,

    1. emacs -Q
    2. M-: (require 'org-id)
    3. Visit any file (M-x find-library RET org-id RET)
    4. M-: (org-id-find "non-existent")
    5. Witness the major-mode change

This all happens in org-id-find-id-in-file where when VISITING is
non-nil, the buffer is (re)used whether or not it was originally in
org-mode.  The problem is complicated by the use of find-file-noselect
when MARKERP argument to org-id-find is non-nil.  Perhaps,
org-id-find-id-file should check if the current-buffer is derived from
org-mode before returning the buffer-file-name. WDYT?

Emacs  : GNU Emacs 31.0.50 (build 7, x86_64-pc-linux-gnu, X toolkit, cairo version 1.18.0, Xaw scroll bars)
 of 2024-09-09
Package: Org mode version 9.8-pre (release_9.7.8-713-g62cbac @ /home/viz/lib/emacs/straight/build/org/)

Re: [BUG] org-id-find might change the major-mode of a buffer [9.8-pre (release_9.7.8-713-g62cbac @ /home/viz/lib/emacs/straight/build/org/)]

[Visuwesh]

[செவ்வாய் செப்டம்பர் 17, 2024] Visuwesh wrote:

> Remember to cover the basics, that is, what you expected to happen and
> what in fact did happen.  You don't know how to make a good report?  See
>
>      https://orgmode.org/manual/Feedback.html#Feedback
>
> Your bug report will be posted to the Org mailing list.
> ------------------------------------------------------------------------
>
> Hello,
>
> I have code in my init that calls org-id-find to get the entry ID refers
> to.  When the ID is non-existent, the function unintentionally changes
> the major-mode of the current buffer.
> [ This almost caused me to lose a PDF file, but thankfully I could
>   recover it because of Emacs backup.  Admittedly this happened because
>   I was half-panicking and half-confused.  ]
>
> To reproduce,
>
>     1. emacs -Q
>     2. M-: (require 'org-id)
>     3. Visit any file (M-x find-library RET org-id RET)
>     4. M-: (org-id-find "non-existent")
>     5. Witness the major-mode change
>
> This all happens in org-id-find-id-in-file where when VISITING is
> non-nil, the buffer is (re)used whether or not it was originally in
> org-mode.  The problem is complicated by the use of find-file-noselect
> when MARKERP argument to org-id-find is non-nil.  Perhaps,
> org-id-find-id-file should check if the current-buffer is derived from
> org-mode before returning the buffer-file-name. WDYT?

The patch below is what I had in mind.  With it, I can no longer
reproduce the recipe above.

diff --git a/lisp/org-id.el b/lisp/org-id.el
index e247fab1d..17568ab31 100644
--- a/lisp/org-id.el
+++ b/lisp/org-id.el
@@ -705,8 +705,9 @@ (defun org-id-find-id-file (id)
 	   (hash-table-p org-id-locations)
 	   (gethash id org-id-locations))
       ;; Fall back on current buffer
-      (buffer-file-name (or (buffer-base-buffer (current-buffer))
-			    (current-buffer)))))
+      (when (derived-mode-p 'org-mode)
+        (buffer-file-name (or (buffer-base-buffer (current-buffer))
+			      (current-buffer))))))
 
 (defun org-id-find-id-in-file (id file &optional markerp)
   "Return the position of the entry ID in FILE.

Re: [BUG] org-id-find might change the major-mode of a buffer [9.8-pre (release_9.7.8-713-g62cbac @ /home/viz/lib/emacs/straight/build/org/)]

[Ihor Radchenko]

Visuwesh <visuweshm@gmail.com> writes:

> The patch below is what I had in mind.  With it, I can no longer
> reproduce the recipe above.
> ...

The patch looks reasonable.
May you format it properly, adding a proper commit message? See
https://orgmode.org/worg/org-contribute.html#commit-messages

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

Re: [BUG] org-id-find might change the major-mode of a buffer [9.8-pre (release_9.7.8-713-g62cbac @ /home/viz/lib/emacs/straight/build/org/)]

[Visuwesh]

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

[ஞாயிறு செப்டம்பர் 22, 2024] Ihor Radchenko wrote:

> Visuwesh <visuweshm@gmail.com> writes:
>
>> The patch below is what I had in mind.  With it, I can no longer
>> reproduce the recipe above.
>> ...
>
> The patch looks reasonable.
> May you format it properly, adding a proper commit message? See
> https://orgmode.org/worg/org-contribute.html#commit-messages

Please find attached.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-org-id-Guard-against-unwarranted-major-mode-change-w.patch --]
[-- Type: text/x-diff, Size: 1284 bytes --]

From 9ac977253620e719b239e08b2ff8aa1082ae8db3 Mon Sep 17 00:00:00 2001
From: Visuwesh <visuweshm@gmail.com>
Date: Sun, 22 Sep 2024 14:38:54 +0530
Subject: [PATCH] org-id: Guard against unwarranted major-mode change when
 querying id

* lisp/org-id.el (org-id-find-id-file): Only fallback to the
current-buffer filename when it is an org-mode buffer.  This avoids
accidental change of the major-mode when querying a non-existent id
from a non-org-mode buffer.

Reported-by: Visuwesh <visuweshm@gmail.com>
Link: https://list.orgmode.org/87ikuuwmz4.fsf@gmail.com/
---
 lisp/org-id.el | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/lisp/org-id.el b/lisp/org-id.el
index e247fab1d..17568ab31 100644
--- a/lisp/org-id.el
+++ b/lisp/org-id.el
@@ -705,8 +705,9 @@ (defun org-id-find-id-file (id)
 	   (hash-table-p org-id-locations)
 	   (gethash id org-id-locations))
       ;; Fall back on current buffer
-      (buffer-file-name (or (buffer-base-buffer (current-buffer))
-			    (current-buffer)))))
+      (when (derived-mode-p 'org-mode)
+        (buffer-file-name (or (buffer-base-buffer (current-buffer))
+			      (current-buffer))))))
 
 (defun org-id-find-id-in-file (id file &optional markerp)
   "Return the position of the entry ID in FILE.
-- 
2.45.2

Re: [BUG] org-id-find might change the major-mode of a buffer [9.8-pre (release_9.7.8-713-g62cbac @ /home/viz/lib/emacs/straight/build/org/)]

[Ihor Radchenko]

Visuwesh <visuweshm@gmail.com> writes:

>> The patch looks reasonable.
>> May you format it properly, adding a proper commit message? See
>> https://orgmode.org/worg/org-contribute.html#commit-messages
>
> Please find attached.

Thanks!
Applied, onto main.
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=5dc7028d7f
Fixed.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

Re: [BUG] org-id-find might change the major-mode of a buffer [9.8-pre (release_9.7.8-713-g62cbac @ /home/viz/lib/emacs/straight/build/org/)]

[Visuwesh]

[ஞாயிறு செப்டம்பர் 22, 2024] Ihor Radchenko wrote:

> Visuwesh <visuweshm@gmail.com> writes:
>
>>> The patch looks reasonable.
>>> May you format it properly, adding a proper commit message? See
>>> https://orgmode.org/worg/org-contribute.html#commit-messages
>>
>> Please find attached.
>
> Thanks!
> Applied, onto main.
> https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=5dc7028d7f
> Fixed.

Thank you!