-
-
Notifications
You must be signed in to change notification settings - Fork 240
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
fix(obsidian-nvim): added obsidian.nvim config and changed the readme #378
Conversation
Review ChecklistDoes this PR follow the [Contribution Guidelines](development guidelines)? Following is a partial checklist: Proper conventional commit scoping:
|
Wouldn't it be enough to just add your 2 changes to the |
Setting up the |
Pardon me then, I came from the issue and the first part as well as the comments seem to be directly taken from the obsidian README. But my point stands, it's unclear for me what parts deviate from the default config if you also put in all the defaults and leave all the comments in. Also we'd have to handle line 91-95 in a way that is OS agnostic. For that we'd have to use a function for opts instead. Lastly we should setup the key bind in a different way than to place it in the |
I agree with both of these points. I admit, however, that I am unfamiliar with how astronvim should handle both of these. If you have solutions for them, please share. |
I can refactor |
Shouldn't it work with the |
Hmm there's still an issue with the setup handler in my code. Testing right now... |
the removed opts are already set to the same values by default, see here: https://github.com/epwalsh/obsidian.nvim/blob/main/lua/obsidian/config.lua
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.
I like my own changes, soo ...
@owittek Added a suggestion for windows support as well. |
Co-authored-by: Uzair Aftab <[email protected]>
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.
the pipeline does not like us
@owittek Apparently |
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.
Co-authored-by: Micah Halter <[email protected]>
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
As requested from #309. We need this config to follow links to other markdown files. Please check if anything is amiss.
Edit: I see the review checklist asks for "Proper usage of
opts
table rather than setting things up with theconfig
function." I'm not sure if that is satisfied here.