nvimtools / hydra.nvim

Create custom submodes and menus
MIT License
145 stars 8 forks source link

feat: set any nvim_open_win option on hint window #6

Closed benlubas closed 10 months ago

benlubas commented 10 months ago

closes #4

Things to note:

miversen33 commented 10 months ago

I like the idea here but this does break existing configurations. Using your branch, I lose my border. I can of course change my config to utilize the new float_opts and it does of course work then. But I think we should be very careful about breaking stuff while moving forward.

Neo-tree handles this by deprecating first and removing later, a tact I have taken on in my own projects as well. I think if we are going to move forward with the idea that float_opts is how we are going to configure the appearance of hydras, we must inform the user first before breaking their configuration. After all, breaking changes are bad

Hydra does not have any sort of deprecation system (that I am aware of), so we would probably want to add that.

Thoughts?

benlubas commented 10 months ago

Yup, definitely agree. I honestly just forgot to keep support for the old config, but had decided it was a good idea at the start.

benlubas commented 10 months ago

Hydra does not have any sort of deprecation system (that I am aware of), so we would probably want to add that.

Adding versioning is probably the way to go, release-please is really nice I can set it up.

As far as notifying users, we can just include a simple check and then use vim.notify() maybe add a global so it only triggers once (per nvim launch)

miversen33 commented 10 months ago

I think versioning is a good idea as well. I have added a deprecation system to hydra but I cannot seem to push to your branch so I guess check out my branch and see if we can merge that onto this. Alternatively, I can merge in the deprecation system and you can rebase onto main after its in place. IMO it should make handling stuff like this in the future better.

Also unrelated to that, we need to update the vimdoc as it still says "hint.border" and has no reference to "hint.float_opts".

persisted.nvim uses a makefile to automagically update their vimdoc based on the readme, I wonder how difficult that would be to add here

miversen33 commented 10 months ago

Deprecation system is merged, do you want to rebase this and use that?

benlubas commented 10 months ago

ah there's an issue, hint can be a boolean, so we need to type check it.

benlubas commented 10 months ago

I'm going to look into the vim doc generation now

benlubas commented 10 months ago

I think it's best to just use the github action instead of setting it up in a makefile.

benlubas commented 10 months ago

Reading through, that kinda clobbers a lot of good headers. I think that's just going to be temporary though, after the readme overhaul it should look a lot better

miversen33 commented 10 months ago

I am going to wait to further review this until all changes are done :) Feel free to ping me once you are good

benlubas commented 10 months ago

@miversen33 I think this is good to review. I'm going to do the readme stuff in a different PR I think, probably in a few parts

miversen33 commented 10 months ago

I appreciate this, thanks for the contributions as well :)