Skip to content
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

Automatically configure 'gf' passthrough #161

Merged
merged 6 commits into from
Jul 26, 2023
Merged

Automatically configure 'gf' passthrough #161

merged 6 commits into from
Jul 26, 2023

Conversation

epwalsh
Copy link
Owner

@epwalsh epwalsh commented Jul 26, 2023

Fixes #157.

  • Adds a mappings field to the config.
  • obsidian.nvim will now automatically configure the 'gf' passthrough unless this keybinding was already set or you override the mappings field in your obsidian.nvim config.

Fixes #157.

obsidian.nvim will now automatically configure the 'gf' pasthrough
unless this keybinding was already set or you put
'keybindings.gf_passthrough = false' in your obsidian.nvim config.
Copy link
Contributor

@shakesbeare shakesbeare left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a few comments

@@ -20,7 +20,7 @@ describe("Note", function()
local note = Note.from_file "README.md"
assert.equals(note.id, "README")
assert.equals(#note.aliases, 1)
assert.equals(note.aliases[1], "Obsidian.nvim")
assert.equals(note.aliases[1], "obsidian.nvim")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like it might be unrelated and might be deserving of its own message in the commit log

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was just because I changed the title of the README from "Obsidian.nvim" to "obsidian.nvim" to be consistent with the repo name.


-- Optional, override the 'gf' keymap to utilize obsidian.nvim's search functionality.
-- see also: 'follow_url_func' config option below.
vim.keymap.set("n", "gf", function()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the spirit of updating the docs, we might want to leave this information somewhere in the README in to provide some direction for users who wish to manage their commands manually. For example, I don't think we document anywhere that this helper function cursor_on_markdown_link() exists, which might be very helpful information for power users.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeup, agreed: 3da669d

if gf_set_by == "" then
-- 'gf' has not been overridden by user or other plugin so we can safely set it.
vim.keymap.set("n", "gf", function()
if require("obsidian").util.cursor_on_markdown_link() then
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there should be an additional check here to ensure that the current buffer is located within the obsidian vault. Could cause problems if someone happens to be writing links formatted like Obsidian links elsewhere.

Maybe it should be an optional parameter for gf_passthrough? only_allow_in_vault or something like that.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we create this keymap within the lazy setup function and we have buffer = true in the keymap options, it's already only going to be set for buffers in the vault.

README.md Outdated
-- Optional, keybindings.
keybindings = {
-- Overrides the 'gf' keybinding to work on markdown/wiki links within your vault.
gf_passthrough = true,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is exactly what I described in my issue suggesting this, but I'm now wondering if a more robust system might be preferable? For instance, like what can be found in here.

Could potentially come up with a system like that for mappings to many of our commands with options (like passthrough) to modify their behavior from default.

One the one hand, it seems a bit more transparent in case a user might want different keybind(s) than we set in the plugin, but it also might be a waste of effort if this is the only bind we ever set up.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually I like that a lot. 53ea312

@epwalsh epwalsh merged commit 00b8950 into main Jul 26, 2023
5 checks passed
@epwalsh epwalsh deleted the auto-gf-passthrough branch July 26, 2023 23:27
flaviosakakibara pushed a commit to flaviosakakibara/obsidian.nvim that referenced this pull request Sep 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make gf keybinding a part of the config
2 participants