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(widgets): quit floats with q #1353

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

igorlfs
Copy link

@igorlfs igorlfs commented Oct 17, 2024

Addresses a suggestion I made in #1194 (comment).

Copy link
Owner

@mfussenegger mfussenegger left a comment

Choose a reason for hiding this comment

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

I'm not sure about this - if every plugin ends up mapping q then this should probably just be a default in neovim core instead for floating windows.

If it isn't a good default for all plugins, then I wonder why it should be done here. Having it work in one plugin but not another is odd.

If you do want to have it, you can register a FileType autocmd on dap-float, or you could even do a WinEnter and check if it's a floating window to set it generally.

@igorlfs
Copy link
Author

igorlfs commented Oct 31, 2024

if every plugin ends up mapping q then this should probably just be a default in neovim core instead for floating windows.

That sounds like a better approach, but I'm unsure if other maintainers would be willing to accept such change (though it's common within core, e.g., for diagnostics and hover). If you're okay with this, I'll file an issue in core.

If it isn't a good default for all plugins, then I wonder why it should be done here. Having it work in one plugin but not another is odd.

IMO it's a good default for all plugins (if not, the vast majority). At least, most of the plugin I use, anyway.

If you do want to have it, you can register a FileType autocmd on dap-float

That's what I currently have, but your second idea sounds better, thanks!

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