[BUG] org-refile-get-location a symlinked file [9.7-pre (release_9.6.29-1443-gb4d17c)]

[BUG] org-refile-get-location a symlinked file [9.7-pre (release_9.6.29-1443-gb4d17c)]

[Al Haji-Ali]

I have `org-refile-use-outline-path` set to t, expecting that when refiling, the filename should not appear in the list. However, because I am opening an org file using a symblink and I have `find-file-visit-truename` set to `t`, the test

`(not (equal filename (nth 1 x)))`

in `org-refile-get-location` always fails and the filename is displayed even when the current buffer and `(nth 1 x)` point to the same file. If I replace this condition by

`(not (equal (file-truename filename) (file-truename (nth 1 x))))`

The filename is not added to the list of refile targets as I would expect.

-- Al

--
Emacs  : GNU Emacs 29.1 (build 1, aarch64-apple-darwin22.5.0, Carbon Version 169 AppKit 2299.6)
 of 2023-09-09
Package: Org mode version 9.7-pre (release_9.6.29-1443-gb4d17c)

Re: [BUG] org-refile-get-location a symlinked file [9.7-pre (release_9.6.29-1443-gb4d17c)]

[Ihor Radchenko]

Al Haji-Ali <abdo.haji.ali@gmail.com> writes:

> I have `org-refile-use-outline-path` set to t, expecting that when refiling, the filename should not appear in the list. However, because I am opening an org file using a symblink and I have `find-file-visit-truename` set to `t`, the test
>
> `(not (equal filename (nth 1 x)))`
>
> in `org-refile-get-location` always fails and the filename is displayed even when the current buffer and `(nth 1 x)` point to the same file. If I replace this condition by
>
> `(not (equal (file-truename filename) (file-truename (nth 1 x))))`
>
> The filename is not added to the list of refile targets as I would expect.

Makes sense.
May you prepare a patch with the proposed change?
See https://orgmode.org/worg/org-contribute.html#first-patch

-- 
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-refile-get-location a symlinked file [9.7-pre (release_9.6.29-1443-gb4d17c)]

[Al Haji-Ali]

On 17/06/2024, Ihor Radchenko wrote:
> Makes sense.
> May you prepare a patch with the proposed change?
> See https://orgmode.org/worg/org-contribute.html#first-patch

It's not really a long patch, just involves calling `file-truename` instead of `expand-file-name` in a couple of places.

diff --git a/lisp/org-refile.el b/lisp/org-refile.el
index 20b5fbd02..7c463bc67 100644
--- a/lisp/org-refile.el
+++ b/lisp/org-refile.el
@@ -324,7 +324,7 @@ When `org-refile-use-cache' is nil, just return POS."
             (progn
               (when (bufferp f)
                 (setq f (buffer-file-name (buffer-base-buffer f))))
-              (setq f (and f (expand-file-name f)))
+              (setq f (and f (file-truename f)))
               (when (eq org-refile-use-outline-path 'file)
                 (push (list (and f (file-name-nondirectory f)) f nil nil) tgs))
               (when (eq org-refile-use-outline-path 'buffer-name)
@@ -666,7 +666,7 @@ this function appends the default value from
                  #'completing-read))
         (extra (if org-refile-use-outline-path "/" ""))
         (cbnex (concat (buffer-name) extra))
-        (filename (and cfn (expand-file-name cfn)))
+        (filename (and cfn (file-truename cfn)))
         (tbl (mapcar
               (lambda (x)
                 (if (and (not (member org-refile-use-outline-path

-- Al

Re: [BUG] org-refile-get-location a symlinked file [9.7-pre (release_9.6.29-1443-gb4d17c)]

[Ihor Radchenko]

Al Haji-Ali <abdo.haji.ali@gmail.com> writes:

> It's not really a long patch, just involves calling `file-truename` instead of `expand-file-name` in a couple of places.
>
> diff --git a/lisp/org-refile.el b/lisp/org-refile.el
> index 20b5fbd02..7c463bc67 100644

Hmm. Thanks, but the patch does not apply on my side.
May you instead create a patch as a separate file, add the commit
message, and attach that patch file in the email reply?

-- 
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-refile-get-location a symlinked file [9.7-pre (release_9.6.29-1443-gb4d17c)]

[Al Haji-Ali]

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


On 18/06/2024, Ihor Radchenko wrote:
> Hmm. Thanks, but the patch does not apply on my side.
> May you instead create a patch as a separate file, add the commit
> message, and attach that patch file in the email reply?

See attached.

Best regards,
-- Al


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: org-refile-targets.patch --]
[-- Type: text/x-patch, Size: 1337 bytes --]

From 0efafc38b404bace9ff87e38349a6c64fc7be903 Mon Sep 17 00:00:00 2001
From: Al Haji-Ali <a.hajiali@hw.ac.uk>
Date: Tue, 18 Jun 2024 19:27:56 +0100
Subject: [PATCH] Use file-truename for files of refile targets.

Ensures that comparing filenames works even when symlinks are used.
---
 lisp/org-refile.el | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lisp/org-refile.el b/lisp/org-refile.el
index 20b5fbd02..7c463bc67 100644
--- a/lisp/org-refile.el
+++ b/lisp/org-refile.el
@@ -324,7 +324,7 @@ When `org-refile-use-cache' is nil, just return POS."
 	     (progn
 	       (when (bufferp f)
 		 (setq f (buffer-file-name (buffer-base-buffer f))))
-	       (setq f (and f (expand-file-name f)))
+	       (setq f (and f (file-truename f)))
 	       (when (eq org-refile-use-outline-path 'file)
 		 (push (list (and f (file-name-nondirectory f)) f nil nil) tgs))
 	       (when (eq org-refile-use-outline-path 'buffer-name)
@@ -666,7 +666,7 @@ this function appends the default value from
 		  #'completing-read))
 	 (extra (if org-refile-use-outline-path "/" ""))
 	 (cbnex (concat (buffer-name) extra))
-	 (filename (and cfn (expand-file-name cfn)))
+	 (filename (and cfn (file-truename cfn)))
 	 (tbl (mapcar
 	       (lambda (x)
 		 (if (and (not (member org-refile-use-outline-path
-- 
2.39.3 (Apple Git-145)

Re: [BUG] org-refile-get-location a symlinked file [9.7-pre (release_9.6.29-1443-gb4d17c)]

[Ihor Radchenko]

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

Al Haji-Ali <abdo.haji.ali@gmail.com> writes:

> On 18/06/2024, Ihor Radchenko wrote:
>> Hmm. Thanks, but the patch does not apply on my side.
>> May you instead create a patch as a separate file, add the commit
>> message, and attach that patch file in the email reply?
>
> See attached.

Thanks. Although this is a different kind of patch. And it is doing too
much - we do not want to expand all the names via `file-truename'
because you may have a situation like

/path/to/file1.org
/path/to/symlinks/name-is-completely-different.org

Then, using `file-truename' may be confusing for people who expect the
symlink name to be displayed.

I am thus attaching an alternative patch.
May you please test if it fixes the problem on your side?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-org-refile-get-location-Fix-outline-path-to-a-symlin.patch --]
[-- Type: text/x-patch, Size: 1371 bytes --]

From 8e0c1dbaa54322029c6cf1389bf5a43340738da2 Mon Sep 17 00:00:00 2001
Message-ID: <8e0c1dbaa54322029c6cf1389bf5a43340738da2.1718806993.git.yantar92@posteo.net>
From: Ihor Radchenko <yantar92@posteo.net>
Date: Wed, 19 Jun 2024 16:18:41 +0200
Subject: [PATCH] org-refile-get-location: Fix outline path to a symlink of
 current buffer

* lisp/org-refile.el (org-refile-get-location): When current buffer
file is a symlink to refile location, do not append the file name to
the outline path, just as we do when current buffer is the same as
refile location file.

Reported-by: Al Haji-Ali <abdo.haji.ali@gmail.com>
Link: https://orgmode.org/list/m2ed8vr2io.fsf@gmail.com
---
 lisp/org-refile.el | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lisp/org-refile.el b/lisp/org-refile.el
index 20b5fbd02..4a9cc8c01 100644
--- a/lisp/org-refile.el
+++ b/lisp/org-refile.el
@@ -670,8 +670,8 @@ (defun org-refile-get-location (&optional prompt default-buffer new-nodes)
 	 (tbl (mapcar
 	       (lambda (x)
 		 (if (and (not (member org-refile-use-outline-path
-				       '(file full-file-path title)))
-			  (not (equal filename (nth 1 x))))
+				     '(file full-file-path title)))
+			  (not (equal filename (file-truename (nth 1 x)))))
 		     (cons (concat (car x) extra " ("
 				   (file-name-nondirectory (nth 1 x)) ")")
 			   (cdr x))
-- 
2.45.1


[-- Attachment #3: Type: text/plain, Size: 224 bytes --]


-- 
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-refile-get-location a symlinked file [9.7-pre (release_9.6.29-1443-gb4d17c)]

[Al Haji-Ali]

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


On 19/06/2024, Ihor Radchenko wrote:

> Al Haji-Ali <abdo.haji.ali@gmail.com> writes:
>
>> On 18/06/2024, Ihor Radchenko wrote:
>>> Hmm. Thanks, but the patch does not apply on my side.
>>> May you instead create a patch as a separate file, add the commit
>>> message, and attach that patch file in the email reply?
>>
>> See attached.
>
> Thanks. Although this is a different kind of patch. And it is doing too
> much - we do not want to expand all the names via `file-truename'
> because you may have a situation like
>
> /path/to/file1.org
> /path/to/symlinks/name-is-completely-different.org
>
> Then, using `file-truename' may be confusing for people who expect the
> symlink name to be displayed.

I am not sure what would be the correct approach. Perhaps the true-name expansion should be conditioned on the value of `find-file-visit-truename`.

> I am thus attaching an alternative patch.
> May you please test if it fixes the problem on your side?
I think it's missing calling `file-truename` call on `filename`, see attached patch.

Best regards,
-- Al

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: org-refile-targets.patch --]
[-- Type: text/x-patch, Size: 1295 bytes --]

From 9b44f737a44945723a9cf35dfa92e19d27a1135b Mon Sep 17 00:00:00 2001
From: Al Haji-Ali <a.hajiali@hw.ac.uk>
Date: Tue, 18 Jun 2024 19:27:56 +0100
Subject: [PATCH] org-refile-get-location: Fix outline path to a symlink of
 current buffer

* lisp/org-refile.el (org-refile-get-location): When current buffer
file is a symlink to refile location, do not append the file name to
the outline path, just as we do when current buffer is the same as
refile location file.
---
 lisp/org-refile.el | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lisp/org-refile.el b/lisp/org-refile.el
index 20b5fbd02..c8e64903c 100644
--- a/lisp/org-refile.el
+++ b/lisp/org-refile.el
@@ -666,12 +666,12 @@ this function appends the default value from
 		  #'completing-read))
 	 (extra (if org-refile-use-outline-path "/" ""))
 	 (cbnex (concat (buffer-name) extra))
-	 (filename (and cfn (expand-file-name cfn)))
+	 (filename (and cfn (file-truename cfn)))
 	 (tbl (mapcar
 	       (lambda (x)
 		 (if (and (not (member org-refile-use-outline-path
 				       '(file full-file-path title)))
-			  (not (equal filename (nth 1 x))))
+			  (not (equal filename (file-truename (nth 1 x)))))
 		     (cons (concat (car x) extra " ("
 				   (file-name-nondirectory (nth 1 x)) ")")
 			   (cdr x))

Re: [BUG] org-refile-get-location a symlinked file [9.7-pre (release_9.6.29-1443-gb4d17c)]

[Ihor Radchenko]

Al Haji-Ali <abdo.haji.ali@gmail.com> writes:

>> I am thus attaching an alternative patch.
>> May you please test if it fixes the problem on your side?
> I think it's missing calling `file-truename` call on `filename`, see attached patch.

Makes sense.
Applied, onto bugfix.
Fixed.
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?h=bugfix&id=a4ff518

>> Thanks. Although this is a different kind of patch. And it is doing too
>> much - we do not want to expand all the names via `file-truename'
>> because you may have a situation like
>>
>> /path/to/file1.org
>> /path/to/symlinks/name-is-completely-different.org
>>
>> Then, using `file-truename' may be confusing for people who expect the
>> symlink name to be displayed.
>
> I am not sure what would be the correct approach. Perhaps the
> true-name expansion should be conditioned on the value of
> `find-file-visit-truename`.

Maybe. I am slightly cautious about adding it without public discussion
though - (1) I can see how people may not expect
`find-file-visit-truename` to affect `org-refile'; (2) I am not 100%
familiar with this area of Org codebase, and I am not 100% confident that
changing refile targets table won't break anything.

If you want to add support of `find-file-visit-truename`, please send
another email marked [FR] (feature request) in subject line, explaining
what you want to add. Then, we can see if more people are interested.

-- 
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-refile-get-location a symlinked file [9.7-pre (release_9.6.29-1443-gb4d17c)]

[Al Haji-Ali]

On 20/06/2024, Ihor Radchenko wrote:
> Al Haji-Ali <abdo.haji.ali@gmail.com> writes:
> Makes sense.
> Applied, onto bugfix.
> Fixed.
> https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?h=bugfix&id=a4ff518
Thanks!
>
> Maybe. I am slightly cautious about adding it without public discussion
> though - (1) I can see how people may not expect
> `find-file-visit-truename` to affect `org-refile'; (2) I am not 100%
> familiar with this area of Org codebase, and I am not 100% confident that
> changing refile targets table won't break anything.

I see your point. I think the fix above address the main issue for me.n
The other change is more from an academic of what is the "correct" behaviour. I don't think it is necessary if it will introduce confusion.

Best regards,
-- Al

Re: [BUG] org-refile-get-location a symlinked file [9.7-pre (release_9.6.29-1443-gb4d17c)]

[Ihor Radchenko]

Al Haji-Ali <abdo.haji.ali@gmail.com> writes:

>> Maybe. I am slightly cautious about adding it without public discussion
>> though - (1) I can see how people may not expect
>> `find-file-visit-truename` to affect `org-refile'; (2) I am not 100%
>> familiar with this area of Org codebase, and I am not 100% confident that
>> changing refile targets table won't break anything.
>
> I see your point. I think the fix above address the main issue for me.n
> The other change is more from an academic of what is the "correct"
> behaviour. I don't think it is necessary if it will introduce
> confusion.

I am not sure if it _will_ introduce confusion. That's why I said that
it may be a good idea to discuss such a feature. Of course, if you deem
this feature as important enough to bother :)

-- 
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>