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

feat(note-taking): Add venn-nvim #901

Merged
merged 4 commits into from
Apr 23, 2024
Merged

feat(note-taking): Add venn-nvim #901

merged 4 commits into from
Apr 23, 2024

Conversation

rameshsanth
Copy link
Contributor

Draw ASCII diagrams easily in Neovim

📑 Description

Added new plugin with necessary configuration per the author

ℹ Additional Information

Copy link

github-actions bot commented Apr 20, 2024

Review Checklist

Does this PR follow the [Contribution Guidelines](development guidelines)? Following is a partial checklist:

Proper conventional commit scoping:

  • If you are adding a new plugin, the scope would be the name of the category it is being added into. ex. feat(utility): added noice.nvim plugin

  • If you are modifying a pre-existing plugin or pack, the scope would be the name of the plugin folder. ex. fix(noice-nvim): fix LSP handler error

  • Pull request title has the appropriate conventional commit type and scope where the scope is the name of the pre-existing directory in the project as described above

  • README is properly formatted and uses fenced in links with <url> unless they are inside a [title](url)

  • Proper usage of opts table rather than setting things up with the config function.

Copy link
Member

@Uzaaft Uzaaft left a comment

Choose a reason for hiding this comment

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

See comments.

@rameshsanth
Copy link
Contributor Author

See comments.

how do you remove mapping. set_mappings seems to only set ?

@rameshsanth rameshsanth requested a review from Uzaaft April 21, 2024 16:06
@mehalter
Copy link
Member

I'll handle this :)

@mehalter
Copy link
Member

@rameshsanth I modified this implementation a bit to integrate a bit more seamlessly with AstroCore and make the mapping more easily configurable by the user. This also makes it a bit easier to start venn mode by setting up a user command :VennToggle.

It's also important to note that this plugin's method of setting up the buffer mappings is pretty bad and comes in and clobbers any previous buffer local mappings such as K which is used for LSP documentation. So any buffer that uses a language server will have at least that mapping borked after toggling venn mode. Not sure the best way to fix this other than caching already existing mappings on enable and restoring them on disable. Not sure if you want to implement that or not. I think as is this is fine enough to merge since it implements exactly what the README for that plugin recommends even though it's a bad recommendation.

@mehalter
Copy link
Member

@Uzaaft this is ready for review

@Uzaaft
Copy link
Member

Uzaaft commented Apr 23, 2024

@Uzaaft this is ready for review

Reviewing this soon

@Uzaaft Uzaaft merged commit 6bb14a2 into AstroNvim:main Apr 23, 2024
12 checks passed
@mehalter
Copy link
Member

@rameshsanth I actually just added Hydra.nvim to AstroCommunity in a way that it can be integrated with other plugins. I modified this config so that if hydra is available it defines the diagramming mode as a submode in Neovim (As described here: https://github.com/anuvyklack/hydra.nvim/wiki/Draw-diagrams)

If you want to try it out simply add hydra from AstroCommunity :)

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.

None yet

3 participants