generate_folder() has incorrect `relative` logic vs generate_path()
doublethefish opened this issue · 1 comments
Plugin information (please complete the following information):
- OS: OSX
- Templater version: head
- Obsidian version: n/a
- Templater settings: n/a
Describe the bug
The docs and code for tp.file.folder(relative=false)
are misleading/incorrect.
folder()
returns a file-relative folder for relative=false
and a vault-absolute path for relative=true
.
This caused some weirdness in my Templated notes where:
- Notes would sometimes (!) be created in the wrong location.
- When notes were created in the wrong location no template (or the wrong template) would be applied.
// in file `Recurring/Daily/Journal/2024-09-05`
// A link to the following day's Daily-Journal or Daily-Note like:
[[<% tp.file.folder() %>/<% tp.date.now("YYYY-MM-DD", 1, tp.file.title, "YYYY-MM-DD") %>| ⏭️ ]]
// would resolve to:
[[Journal/2024-09-05| ⏭️ ]] // <-- sometime creating the Journal dir in the root of my vault!
// instead of:
[[Recurring/Daily/Journal/2024-09-05| ⏭️ ]] // <-- sometimes creating the Journal dir in the root of my vault!
The above snippet is imported/used/shared by multiple templates, so the folder()
call is quite important./
The issue is quite obvious when you compare folder()
and file()
code:
// src/core/functions/internal_functions/file/InternalModuleFile.ts
// tp.file.folder() - incorrect intended behaviour
generate_folder(): (relative?: boolean) => string {
// ... snip ...
if (relative) {
folder = parent.path; // abs for relative 👎
} else {
folder = parent.name; // relative for abs 👎
}
// ... snip ...
}
// tp.file.path() - correct intended behaviour
generate_path(): (relative: boolean) => string {
// ... snip ...
if (relative) {
return this.config.target_file.path; // relative for relative👍
} else {
return `${vault_path}/${this.config.target_file.path}`; // abs for abs 👍
}
// ... snip ...
}
Expected behaviour
relative=true
to return Note-relative folder paths, and relative=false
to return vault-absolute paths.
Screenshots
n/a
Additional context
Changing or "fixing" the behaviour of the folder()
API would break back-compat and be a maintenance nightmare, but renaming the value and updating the docs would only affect the docs and improve clarity. However, whilst that is a best-fit for effort/reward, the API itself would be more inconsistent. One possible, and more professional option might be to create a new function dirname()
(which would match pathlib
and be less 'surprising' overall, or something better?) and deprecate folder()
, but is it worth the effort?
If we want a more consistent API, we could keep folder()
as is and add a new method as you suggested, and have folder()
call the new method under the hood. No one has to update their templates, and the documentation can communicate that it's deprecated or no longer recommended.
I definitely don't ever want to remove folder()
, that will be a huge pain for me.
Your current PR is definitely a good enough solution, it'll mostly be developers that care about consistent APIs, and I'm guessing most Templater users are not developers.