Skip to content

Conversation

@Faria22
Copy link
Contributor

@Faria22 Faria22 commented Sep 30, 2025

  • remove the register_keymaps toggle and make default conflict mappings install on every setup()
  • allow per-key override/disable via opts.keymaps, with tests stubbing vim.keymap.set to cover defaults and customisations
  • trim the lazy.nvim install example and document the built-in keymaps separately

@StackInTheWild StackInTheWild requested a review from Copilot October 6, 2025 11:48
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Removes the register_keymaps toggle and ensures default keymaps are always registered during setup, while allowing individual keymap customization or disabling through the opts.keymaps configuration.

  • Eliminates the register_keymaps boolean configuration option
  • Implements per-key override/disable functionality via opts.keymaps
  • Adds comprehensive test coverage for keymap registration with stubbed vim.keymap.set

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
lua/headhunter/init.lua Removes register_keymaps toggle, adds quickfix functionality, and implements per-key keymap control
lua/tests/headhunter_spec.lua Adds keymap registration tests with stubbing and removes old toggle-based tests
lua/tests/quickfix_spec.lua New test file for quickfix integration functionality
lua/tests/conflict_resolution_spec.lua Adds test for stash-style conflict marker handling
README.md Updates documentation to reflect keymap changes and adds default keymap reference

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@StackInTheWild
Copy link
Owner

I'm not sure why to always register on setup. Someone told me it wasn't a good practice with Lazy. I'll have to check or perhaps you can explain

@Faria22
Copy link
Contributor Author

Faria22 commented Oct 6, 2025

I'm not sure, I never head that that was a bad practice, and I've seen many plugins do it. I'm just updating a few things that I just noticed were broken.

@Faria22 Faria22 marked this pull request as draft October 6, 2025 13:37
@Faria22 Faria22 marked this pull request as ready for review October 6, 2025 14:12
@Faria22
Copy link
Contributor Author

Faria22 commented Oct 6, 2025

So what I changed is that now it uses the setup method, and I added a "enabled" boolean to toggle the plugin. Now it also makes it easier just to overwrite the default key mappings. I will be honestly, this is pretty different from what you originally have, but I think this is more in line with what other plugins use.

@Faria22 Faria22 requested a review from Copilot October 6, 2025 23:43
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.


Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@StackInTheWild
Copy link
Owner

So what I changed is that now it uses the setup method, and I added a "enabled" boolean to toggle the plugin. Now it also makes it easier just to overwrite the default key mappings. I will be honestly, this is pretty different from what you originally have, but I think this is more in line with what other plugins use.

I'll take a look when I have time, probably in a day or two

@StackInTheWild StackInTheWild merged commit de8b666 into StackInTheWild:main Oct 21, 2025
@Faria22 Faria22 deleted the readme branch November 3, 2025 14:54
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.

2 participants