"No links found" when using `denote-find-link` in denote files inside a silo

[Kolmas225]

Steps to reproduce:

1. Open any denote file with links to another file inside the same silo
2. M-x denote-find-link
3. "No links found" shows up in stead of a list of forward links of current file

[protesilaos]

From: Kolmas @.***> Date: Tue, 27 Aug 2024 04:04:45 -0700

Steps to reproduce:

1. Open any denote file with links to another file inside the same silo
2. M-x denote-find-link
3. "No links found" shows up in stead of a list of forward links of current file

Thank you for reporting this! I can reproduce it and will work on it now.

-- Protesilaos Stavrou https://protesilaos.com

[protesilaos]

I just pushed a change. Please let me know if this works for you.

Even if it does work, I suspect there is something deeper going on. We have the same problem with the denote-find-backlink command, so I wonder if the core issue here is how the function denote-directory works when the silo is defined as a string. This is a change we made fairly recently (cc @jeanphilippegg). In a debug I was doing, the denote-directory function did return the silo for the first file in the list, but then it reverted to the non-silo value. I need to explore this further.

[protesilaos]

Indeed, if I set the local value to a non-string like we had in the past, then denote-find-backlink works as expected. The .dir-local.el:

((nil . (;; (denote-directory . "/home/prot/Documents/denote-test-silo/")
         (denote-directory . local))))

[Kolmas225]

I just pushed a change. Please let me know if this works for you.

It works as intended, thank you for the quick fix!

[jeanphilippegg]

I think I have found out the cause the of issue. Execute the following function inside of a silo and test the commented lines:

(defun test-dir-locals-el ()
  (interactive)
  (message "Before with-*-buffer: %s" denote-directory)
  (with-current-buffer (get-buffer-create "test")   ; <---- .dir-locals is forgotten
  ;;(with-current-buffer (find-file-noselect (buffer-file-name)) ; <---- WORKS!
  ;;(with-temp-buffer                       ; <---- .dir-locals is forgotten
    (message "Within with-*-buffer: %s" denote-directory))
  (message "After with-*-buffer: %s" denote-directory))

.dir-locals.el variables are forgotten inside a with-temp-buffer block. The same applies with with-current-buffer, but only when the specified buffer is not a buffer within the silo.

In denote-find-link, we were in the with-temp-buffer case. In denote-link--prepare-backlinks, we are in the with-current-buffer case with a temporary buffer.

In addition to the above, (denote-backlinks-mode) also reset buffer-local variables.

The solution

Always do computations that depend on variables that may be specified in .dir-locals.el outside of a with-temp-buffer block and similarly for with-current-buffer. We don't have any problem when we use with-current-buffer with a buffer that is inside of the silo.

I checked all occurrences of these blocks and the issue should be limited to denote-find-link and denote-link--prepare-backlinks. I will check over the weekend if what we have now is correct and how we can improve it. Feel free to make changes until then.

[protesilaos]

From: Jean-Philippe Gagné Guay @.***> Date: Wed, 28 Aug 2024 19:34:53 -0700

I think I have found out the cause the of issue. Execute the following function inside of a silo and test the commented lines:

(defun test-dir-locals-el ()
  (interactive)
  (message "Before with-*-buffer: %s" denote-directory)
  (with-current-buffer (get-buffer-create "test")   ; <---- .dir-locals is forgotten
  ;;(with-current-buffer (find-file-noselect (buffer-file-name)) ; <---- WORKS!
  ;;(with-temp-buffer                       ; <---- .dir-locals is forgotten
    (message "Within with-*-buffer: %s" denote-directory))
  (message "After with-*-buffer: %s" denote-directory))

.dir-locals.el variables are forgotten inside a with-temp-buffer block. The same applies with with-current-buffer, but only when the specified buffer is not a buffer within the silo.

In denote-find-link, we were in the with-temp-buffer case. In denote-link--prepare-backlinks, we are in the with-current-buffer case with a temporary buffer.

Gotcha!

In addition to the above, (denote-backlinks-mode) also reset buffer-local variables.

Yes, this I remember. Now we have the 'denote-link--prepare-backlinks' setting the 'denote-directory' which works but feels wrong: it shoudl just respect all the local variables.

The solution

Always do computations that depend on variables that may be specified in .dir-locals.el outside of a with-temp-buffer block and similarly for with-current-buffer. We don't have any problem when we use with-current-buffer with a buffer that is inside of the silo.

The question then is whether we copy all local variables, as in 'buffer-local-variables', or only the ones we explicitly declare. In the latter scenario, we have a clear idea of the outcome, though we may be causing something to break in a user's setup. Still, this is better than not having Denote work as intended.

I checked all occurrences of these blocks and the issue should be limited to denote-find-link and denote-link--prepare-backlinks. I will check over the weekend if what we have now is correct and how we can improve it. Feel free to make changes until then.

Thank you! I am happy to wait---no pressure. I think this may make sense as a macro, kind of what we have with 'denote--file-with-temp-buffer'. But whatever you choose is okay.

-- Protesilaos Stavrou https://protesilaos.com

[jeanphilippegg]

The question then is whether we copy all local variables, as in 'buffer-local-variables', or only the ones we explicitly declare. In the latter scenario, we have a clear idea of the outcome, though we may be causing something to break in a user's setup. Still, this is better than not having Denote work as intended.

I don't think that we need to set anything explicitly or create a macro. In fact, it makes sense that when we are leaving the directory where there is a .dir-locals.el, then we lose the values of the variables that it sets. It may be just something that we have to keep in mind.

In general, when we need to use with-temp-buffer, we should try to make it a short block that does not do much computation (it must have been done before). See for example, the new denote-link-return-links. We just get what we need in the temp buffer without using any important denote variables and leave the block.

If we really need to access variables from inside of the temp buffer, we can declare a variable like we did in denote-link--prepare-backlinks. We have only needed to do this for that function so far.

[protesilaos]

From: Jean-Philippe Gagné Guay @.***> Date: Fri, 30 Aug 2024 20:15:21 -0700

The question then is whether we copy all local variables, as in 'buffer-local-variables', or only the ones we explicitly declare. In the latter scenario, we have a clear idea of the outcome, though we may be causing something to break in a user's setup. Still, this is better than not having Denote work as intended.

I don't think that we need to set anything explicitly or create a macro. In fact, it makes sense that when we are leaving the directory where there is a .dir-locals.el, then we lose the values of the variables that it sets. It may be just something that we have to keep in mind.

Fine, this sounds safer. I follow your lead!

-- Protesilaos Stavrou https://protesilaos.com

[protesilaos]

@Kolmas225 can you please pull in the latest changes and try again? With @jeanphilippegg contributions we should be done with this.

[Kolmas225]

@Kolmas225 can you please pull in the latest changes and try again? With @jeanphilippegg contributions we should be done with this.

Yes, I'm on 4eaa943f1b97db7fe67479fe915d5a54be0c0447 right now and haven't encounter the problem since.