Skip to content

feat(mini.jump): Add idle timeout to stop jumping automatically#56

Merged
echasnovski merged 4 commits intonvim-mini:mainfrom
annenpolka:main
Mar 25, 2022
Merged

feat(mini.jump): Add idle timeout to stop jumping automatically#56
echasnovski merged 4 commits intonvim-mini:mainfrom
annenpolka:main

Conversation

@annenpolka
Copy link
Contributor

@annenpolka annenpolka commented Mar 25, 2022

This PR adds timeout after idle feature, inspired by lightspeed.nvim.
With this feature, you can reset highlights automatically, and start searching for another character easily.

The default value is set to 10^7, so it won't break behavior compatibility.

@annenpolka annenpolka marked this pull request as ready for review March 25, 2022 02:33
Copy link
Member

@echasnovski echasnovski left a comment

Choose a reason for hiding this comment

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

This is great! Both idea and execution.

Couple of remarks:

  • Please, generate documentation automatically from annotation comments. Inside Neovim run :luafile scripts/minidoc.lua.
  • For a better consistency across modules, I think it is better to not introduce "timeout" notion along with "delay". The best solution here seems to make a breaking change: soft deprecate highlight_delay and introduce delay = { highlight = 250, idle_stop = 10000000 }. This means (at least) updating MiniJump.config (both with documentation), H.setup_config() (update validation logic and notification message if user actually supplied highlight_delay), and all usages of highlight_delay. If this feels a bit overwhelming, I am happy to do it myself (either by pushing to this PR or merging this to a different branch and continue there). If not, I am also happy to guide through this process.

@annenpolka
Copy link
Contributor Author

Firstly, thank you for your kind and in-depth review, and I'm so sorry I overlooked the info about document generation. I'm sure I have made you feel uncomfortable.

I made the changes you reviewed.
I'm a noob developer and bad at English, so there may be some mistakes in both implementation and document wording.

@annenpolka annenpolka requested a review from echasnovski March 25, 2022 13:45
@echasnovski
Copy link
Member

No "uncomfortable" on may end: it is a great PR with useful feature, attention to both details (apart from automatic documentation generation :) ) and code style.

There is some refactoring I would like to do here, but I won't bother you with this.

Thanks for PR!

@echasnovski echasnovski merged commit 8924d0e into nvim-mini:main Mar 25, 2022
@echasnovski echasnovski removed their request for review March 25, 2022 17:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants