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

fix(trouble-nvim): prevent clashes with existing <Leader>x mappings #1105

Merged
merged 1 commit into from
Jul 13, 2024

Conversation

rami3l
Copy link
Contributor

@rami3l rami3l commented Jul 13, 2024

📑 Description

This is to avoid clashes with the recent addition of <Leader>x to AstroNvim.

This PR has also simplified some mapping descriptions.

Copy link

github-actions bot commented Jul 13, 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)

  • Entry returns a single plugin spec with the new plugin as the only top level spec (not applicable for recipes or packs).

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

  • Proper usage of specs table for all specs that are not dependencies of a given plugin (not applicable for recipes or packs).

@rami3l rami3l force-pushed the patch-1 branch 2 times, most recently from 03cab3f to b303c45 Compare July 13, 2024 02:36
@mehalter
Copy link
Member

We can leave all the mappings as is, just change <Leader>xq to <Leader>xQ and <Leader>xl to <Leader>xL

@mehalter
Copy link
Member

No need to make them all 3 key presses when they can share the Quickfix/Lists menu

@rami3l rami3l changed the title fix(trouble-nvim): move mappings to <Leader>xt fix(trouble-nvim): prevent clashes with existing <Leader>x mappings Jul 13, 2024
@rami3l
Copy link
Contributor Author

rami3l commented Jul 13, 2024

We can leave all the mappings as is, just change <Leader>xq to <Leader>xQ and <Leader>xl to <Leader>xL

@mehalter Thanks for your suggestion! I'll modify my PR accordingly.

@rami3l
Copy link
Contributor Author

rami3l commented Jul 13, 2024

We can leave all the mappings as is, just change <Leader>xq to <Leader>xQ and <Leader>xl to <Leader>xL

@mehalter What if other types of Quickfix/Lists show up later though? Looks like we are prioritizing trouble somehow, although that would be beneficial for my own usecase...

@mehalter
Copy link
Member

Yeah I think if the user adds several list plugins it can just be up to them the priority they want

@rami3l
Copy link
Contributor Author

rami3l commented Jul 13, 2024

Yeah I think if the user adds several list plugins it can just be up to them the priority they want

@mehalter Okay, I've applied your suggestions. One problem I have is that the original descriptions are too long, so I've tried my best to shorten them a bit.

Copy link
Member

@mehalter mehalter left a comment

Choose a reason for hiding this comment

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

We should leave the original messages no? That mention Trouble?

@mehalter
Copy link
Member

@rami3l this should be a 2 character change PR

This is to avoid clashes with the [recent addition of `<Leader>x` to AstroNvim](AstroNvim/AstroNvim@99c2b13).
@mehalter
Copy link
Member

Ah I see your most recent comment now

Copy link
Member

@mehalter mehalter left a comment

Choose a reason for hiding this comment

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

Sure! Looks good

@mehalter mehalter merged commit 081a98b into AstroNvim:main Jul 13, 2024
14 checks passed
@rami3l rami3l deleted the patch-1 branch July 13, 2024 14:15
@rami3l
Copy link
Contributor Author

rami3l commented Jul 13, 2024

Sure! Looks good

@mehalter Thanks a lot, and sorry for the previous confusion! 🙇

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