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: extra keybindings override base #224

Merged

Conversation

Valentin271
Copy link
Contributor

@Valentin271 Valentin271 commented Jan 26, 2024

Fixes #188

I suspect this PR will be easier to review by reviewing commits individually (that's why I left them) to understand the reasoning behind the changes. I also left some PR comments to explain some changes.

I refactored the code a bit to be easier to work with, hope that's okay with you.

Copy link
Collaborator

@CosmicHorrorDev CosmicHorrorDev left a comment

Choose a reason for hiding this comment

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

Thanks! Really happy with all of this :)

In general refactors are AOK as long as refactors outside the parts covered by the test suite stay relatively small since it's still really easy to regress things without realizing. I'd also love to get enough test coverage to do larger rewrites of things like the interpreter

@CosmicHorrorDev CosmicHorrorDev added C-enhancement Category: New feature or request A-keybinding Area: Related to keycombos/keybindings A-config Area: Config related issues/PRs labels Jan 27, 2024
@Valentin271
Copy link
Contributor Author

Updated to add logs.

Thanks I'm really happy with how the implementation turned out too.
Understood for the refactors 👍

@CosmicHorrorDev CosmicHorrorDev merged commit a038ae9 into Inlyne-Project:main Jan 27, 2024
9 checks passed
@Valentin271 Valentin271 deleted the feat/extra-precedes-base branch January 27, 2024 21:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-config Area: Config related issues/PRs A-keybinding Area: Related to keycombos/keybindings C-enhancement Category: New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

extra keybindings should have precedence over base
2 participants