-
Notifications
You must be signed in to change notification settings - Fork 8
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
Add configurable buffer_alternate_command option #23
base: master
Are you sure you want to change the base?
Conversation
buffers.kak
Outdated
define-command buffer-alternate -docstring 'open the alternate or previous file in a new buffer' %{ | ||
try %{ | ||
# attempt to execute a custom command specified by the `buffer_alternate_command` option | ||
evaluate-commands %sh{printf "%s\n" "$kak_opt_buffer_alternate_command"} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this patch, I have some complaints ;)
- I think making features require configuration is not really appropriate
for plugin. I like it because I can install it, add a mapping for
: enter-user-mode buffers<ret>
, and I'm done. Plus, this behavior is already
easy to set up with a quickmap global buffers a ': alt<ret>'
. :alt
toggles between test/implementation or .c/.h file. OTOH,ga
switches to the last buffer. I use both of them, so I'd like to bind both.
I've resorted to typing :alt
but maybe we can add these mappings?
map global buffers a 'ga' -docstring 'alternate buffer ↔'
map global buffers t ': alt<ret>' -docstring 'alternate file ↔'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the response!
I can appreciate not wanting to add configuration for the plugin, and recognize that I can accomplish what I'm after fairly easily in my local config. So then is there anything left to change in the plugin itself?
Bottom line: I think ba
should exec : alt<ret>
instead of ga
since it's just as easy to use ga
to accomplish ga
. That way, ba
and ga
each do something useful and different (there are only so many 2-key combos available).
I should mention 3 primary motives for this PR:
- The included
ba
keybinding provided by the buffers user mode is redundant to the built-inga
key binding. And there is another "previous" buffers mapping,bp
. - I think that the term "alternate" is misleading and inaccurate if it really means "previous", or "last".
- The
:alt
alias exists for multiple file types, and I think it makes sense for the buffers user-mode to take advantage of this.
My opinionated change would be to modify the ba
mapping to use the alt
command instead of ga
. Adding a new mapping instead, like you suggest, would be another valid way to utilize the existence of :alt
in this plugin, though I'd just as soon change ba
in my own config.
In either case, there are 2 follow up questions to consider:
- What should happen if the
alt
is not defined? - What should happen If
alt
is defined, but throws an error (no alternate file found or something)?
If in a context in which no alt
command exists, then ga
is a possible fallback that should always work. The reason I added a custom command for ba
mapping instead of just : alt<ret>
was to define this behavior. However, I could see someone wanting to just see the echo'd error message instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see, that all makes sense! Changing the binding to always run :alt
sounds promising (I'd make it error if unset).
ga
should probably still be in there, for discoverability (?). The descriptions "last" and "previous" are already taken, so maybe something like "toggle".. I'm not sure
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I updated the PR to include a simplified buffers-alternate
command without any configuration options.
It now attempts the :alt
command on ba
. If that errors, it then uses ga
instead, which would happen if :alt
is not defined or if the target alt file doesn't exist). I'm not sure I think this is the ideal behavior, but it's the next step.
A few questions:
- What should happen if the
:alt
command isn't defined? - What should happen if the
:alt
command errors (probably because the alternate file doesn't exist). - What should the mapping be for this?
- As previously stated, my intent is to update the
ba
mapping to what [I think] it should be. - Alternatively, a new mapping could be added. I don't know that I can improve on the
bt
you mentioned. or A could be used, but the extra modifier isn't great.
- As previously stated, my intent is to update the
Remove configuration option
The configurable buffer-alternate command option was removed.
The
buffers
user-mode provides thea
key mapping to switch to an alternate buffer, but it is currently just a pass-through toga
.This PR adds a configurable option for setting a custom command for switching to an alternate buffer. The intended use case is to provide a command to switch to a test or spec file as the alternate file, if one exists, rather than the previous buffer.
While looking through the filetype scripts bundled with Kakoune I noticed that more than one filetype script provides an alternate file command along with an 'alt' alias. I think it would be a good idea to simply attempt the 'alt' command alias as a default for the 'a' mapping, with
ga
as a fallback because it would Just Work for certain file types without changing the behavior for others. However, I went the route of leaving the default behavior unchanged and up to the user, while suggesting this in the README instead.