-
-
Notifications
You must be signed in to change notification settings - Fork 38
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
notes/obsidian: remove default dir-value #545
notes/obsidian: remove default dir-value #545
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. One small suggestion, but it's not strictly necessary.
Please do add an entry to the changelog before I can merge this.
Added to changelog, hopefully written correctly |
default = "~/my-vault"; | ||
type = nullOr str; | ||
example = "~/my-vault"; | ||
default = null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does obsidian.nvim
handle this properly?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does
obsidian.nvim
handle this properly?
Yes, from my testing, although i accidentally commited it. Reverting it, as the other option seems better
2ab93dc
to
6852120
Compare
Merging once CI passes. |
thanks :) |
As the
obsidian.nvim
readme describes, thedir
option is provided for backwards compatibility. The problem with this is that becausenvf
defines this value (with a default value), means that it overrides theworkspaces
option, making it impossible to use. Removing it would keep the same functionality, just without the default.