>> Notwithstanding the ongoing discussion, please try commit 3d2e1d5. It> changes the following function to its current form:>> (defun denote--silo-p (path)> "Return path to silo if PATH is a silo."> (when-let (((and path (file-directory-p path)))> (dir-locals (dir-locals-find-file path)))> (cond> ((listp dir-locals)> (car dir-locals))> ((stringp dir-locals)> dir-locals)> (t nil))))
Hi Prot,
It seems the issue I mentioned in https://lists.sr.ht/~protesilaos/denote/%3C80CBB671-D812-4EA8-8C80-85F9F4144051%40disroot.org%3E remains.
The function denote--silo-p will return true for directories containing .dir-locals.el, no matter if the .dir-locals.el contains denote-directory variable or not.
For example, a non silo directory:
~/different/non-silo
|-- .dir-locals.el
|-- foo.txt
|-- bar.el
The .dir-locals.el is an empty file.
(denote--silo-p default-directory) returns true in this folder. And as a result, (denote-directory) returns "~/different/non-silo” instead of the global denote-directory value.
--
Hilde Rhyne
> From: Hilde Rhyne <hilde.rhyne@disroot.org>> Date: Mon, 20 Feb 2023 17:55:52 +0800>>>>> Notwithstanding the ongoing discussion, please try commit 3d2e1d5. It>> changes the following function to its current form:>>>> (defun denote--silo-p (path)>> "Return path to silo if PATH is a silo.">> (when-let (((and path (file-directory-p path)))>> (dir-locals (dir-locals-find-file path)))>> (cond>> ((listp dir-locals)>> (car dir-locals))>> ((stringp dir-locals)>> dir-locals)>> (t nil))))>>> Hi Prot,
Hello Hilde,
> It seems the issue I mentioned in> https://lists.sr.ht/~protesilaos/denote/%3C80CBB671-D812-4EA8-8C80-85F9F4144051%40disroot.org%3E> remains.> [... 17 lines elided]
Does this fix the issue?
(defun denote--silo-p (path)
"Return path to silo if PATH is a silo."
(when-let (((and path (file-directory-p path)))
(dir-locals (dir-locals-find-file path))
((alist-get 'denote-directory dir-local-variables-alist)))
(cond
((listp dir-locals)
(car dir-locals))
((stringp dir-locals)
dir-locals)
(t nil))))
All the best,
Prot
--
Protesilaos Stavrou
https://protesilaos.com
Hi Prot,
thanks for your reply.
> Does this fix the issue?>> (defun denote--silo-p (path)> "Return path to silo if PATH is a silo."> (when-let (((and path (file-directory-p path)))> (dir-locals (dir-locals-find-file path))> ((alist-get 'denote-directory dir-local-variables-alist)))> (cond> ((listp dir-locals)> (car dir-locals))> ((stringp dir-locals)> dir-locals)> (t nil))))>
This patch does fix function denote-directory behavior. But in my
opinion, the function denote--silo-p itself still failed to do what it
claimed to do. The return value depends on dir-local-variables-alist,
which varies from the buffers.
For example, There are two folders, Foo is a silo, and Bar is a non
silo.
~/Documents/Foo ~/Documents/Bar
|-- .dir-locals.el |-- .dir-locals.el
|-- a.org |-- c.org
|-- b.org |-- d.org
The result of (denote--silo-p (expand-file-name "~/Documents/Foo/")) is
the path of Foo when executed in folder Foo, but nil in folder Bar.
regards
Hilde Rhyne
> From: Hilde Rhyne <hilde.rhyne@disroot.org>> Date: Tue, 21 Feb 2023 00:12:57 +0800>> Hi Prot,
Hello Hilde,
> thanks for your reply.
You are welcome! And thank you for taking the time to help me with
this.
>> Does this fix the issue?>>>> (defun denote--silo-p (path)>> "Return path to silo if PATH is a silo.">> (when-let (((and path (file-directory-p path)))>> (dir-locals (dir-locals-find-file path))>> ((alist-get 'denote-directory dir-local-variables-alist)))>> (cond>> ((listp dir-locals)>> (car dir-locals))>> ((stringp dir-locals)>> dir-locals)>> (t nil))))>>>> This patch does fix function denote-directory behavior. But in my> opinion, the function denote--silo-p itself still failed to do what it> claimed to do. The return value depends on dir-local-variables-alist,> which varies from the buffers.>> For example, There are two folders, Foo is a silo, and Bar is a non> silo.>> ~/Documents/Foo ~/Documents/Bar> |-- .dir-locals.el |-- .dir-locals.el> |-- a.org |-- c.org> |-- b.org |-- d.org>>> The result of (denote--silo-p (expand-file-name "~/Documents/Foo/")) is> the path of Foo when executed in folder Foo, but nil in folder Bar.
I think I am misunderstanding you. You say Foo is a silo and the
function returns the path to it. Whereas Bar is not a silo and the
function returns nil. To me this is the desired behaviour: a nil value
means that we fall back to the value of 'denote-directory'.
I put the complete code here for reference:
(defun denote--silo-p (path)
"Return path to silo if PATH is a silo."
(when-let (((and path (file-directory-p path)))
(dir-locals (dir-locals-find-file path))
((alist-get 'denote-directory dir-local-variables-alist)))
(cond
((listp dir-locals)
(car dir-locals))
((stringp dir-locals)
dir-locals)
(t nil))))
(defun denote-directory ()
"Return path of variable `denote-directory' as a proper directory."
(let ((path (or (denote--silo-p default-directory)
(when (and (stringp denote-directory)
(not (file-directory-p denote-directory)))
(make-directory denote-directory t))
(default-value 'denote-directory))))
(file-name-as-directory (expand-file-name path))))
All the best,
Prot
--
Protesilaos Stavrou
https://protesilaos.com
Hi Prot,
thanks for your patience and detailed explanation.
> I think I am misunderstanding you.
Sorry for my pool English, I didn't explain the scenario clear.
>> The result of (denote--silo-p (expand-file-name "~/Documents/Foo/")) is>> the path of Foo when executed in folder Foo, but nil in folder Bar.
What I mean is that when we eval (denote--silo-p (expand-file-name
"~/Documents/Foo/")) in both Foo folder and Bar folder, we will get
different result. And that conflicts with the docstring "Return path to
silo if PATH is a silo". Since Foo folder is a silo,
(denote--silo-p (expand-file-name "~/Documents/Foo/")) should always
return the silo path.
If denote--silo-p is desired to use only in function denote-directory,
then current implementation is completely fine. perhaps we could change
the name and doctring of denote--silo-p to dispell the misconception.
Regards,
Hilde Rhyne
> From: Hilde Rhyne <hilde.rhyne@disroot.org>> Date: Tue, 21 Feb 2023 03:16:05 +0800>> Hi Prot,
Hello Hilde,
> thanks for your patience and detailed explanation.
You are welcome!
>> I think I am misunderstanding you.> Sorry for my pool English, I didn't explain the scenario clear.
No worries!
>>> The result of (denote--silo-p (expand-file-name "~/Documents/Foo/")) is>>> the path of Foo when executed in folder Foo, but nil in folder Bar.>> What I mean is that when we eval (denote--silo-p (expand-file-name> "~/Documents/Foo/")) in both Foo folder and Bar folder, we will get> different result. And that conflicts with the docstring "Return path to> silo if PATH is a silo". Since Foo folder is a silo,> (denote--silo-p (expand-file-name "~/Documents/Foo/")) should always> return the silo path.>> If denote--silo-p is desired to use only in function denote-directory,> then current implementation is completely fine. perhaps we could change> the name and doctring of denote--silo-p to dispell the misconception.
Oh yes, you are right! It retains the old behaviour where it would
actually check for the path. I was focused on getting it to work and
forgot to update that... Now I will change it to only read from the
'default-directory', as this is internal to the workings of the
'denote-directory' function.
Like this:
(defun denote--default-directory-is-silo-p ()
"Return path to silo if `default-directory' is a silo."
(when-let ((dir-locals (dir-locals-find-file default-directory))
((alist-get 'denote-directory dir-local-variables-alist)))
(cond
((listp dir-locals)
(car dir-locals))
((stringp dir-locals)
dir-locals)
(t nil))))
(defun denote-directory ()
"Return path of variable `denote-directory' as a proper directory."
(let ((path (or (denote--default-directory-is-silo-p)
(when (and (stringp denote-directory)
(not (file-directory-p denote-directory)))
(make-directory denote-directory t))
(default-value 'denote-directory))))
(file-name-as-directory (expand-file-name path))))
Thank you!
All the best,
Prot
--
Protesilaos Stavrou
https://protesilaos.com