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

Adding ability to split entry with keybinding #9

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

saccarosium
Copy link

Related to #8

I'm quite happy with how it function now.

I just needed to check if do we maintain the ...Split variants of the command?

@bfrg
Copy link
Owner

bfrg commented Sep 28, 2024

Thanks for the PR. I'm not sure if I want to get rid of the old behavior. Maybe an option can be added where users can decide the behavior.

There's also a mapping missing for opening the selection in a new tab. Maybe <C-t>?

And I don't like polluting the global namespace g: with a variable just for the purpose of setting a temporary value that is used somewhere in the plugin.

I need to think about it.

@saccarosium
Copy link
Author

And I don't like polluting the global namespace g: with a variable just for the purpose of setting a temporary value that is used somewhere in the plugin.

TBH I don't like it either. But I wanted to get your advise.

There's also a mapping missing for opening the selection in a new tab. Maybe ?

Yes, I will include this.

I'm not sure if I want to get rid of the old behavior.

This is only because it would be easier to remove it but I know you are using them

@bfrg
Copy link
Owner

bfrg commented Oct 3, 2024

It seems like :FzyOldfiles doesn't work anymore with this PR. And wouldn't it be convenient if users could specify the mappings for opening in a new split, vsplit or tab? What if someone wanted to use Alt instead of Ctrl?

@saccarosium
Copy link
Author

I've re-did the changes. Now should work all well and be retro compatible with ...Split commands.

Let me know if there is something else.

@saccarosium saccarosium marked this pull request as ready for review October 7, 2024 14:01
@saccarosium
Copy link
Author

This should be ready for merging let me know if there is anything else.

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