-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
refactor(toggleterm)!: simplified keybinds with removed abstraction #3812
base: master
Are you sure you want to change the base?
Conversation
0da8060
to
4e6f588
Compare
@LostNeophyte How do you think we should handle deprecation here? |
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'm not sure we want to add a breaking change to tweak this wrapper, as we mostly use this table as only to alter some common defaults.
At the very least, we can soft-deprecate the original table without doing a breaking change.
Alternatively, we can leave this as is, and suggest using the full API from toggle-term instead.
bfa35aa
to
f3f7301
Compare
term_opts.direction = term_opts.direction or lvim.builtin.terminal.terminals_defaults.direction | ||
term_opts.size = term_opts.size or lvim.builtin.terminal.terminals_defaults[term_opts.direction .. "_size"] | ||
-- size is calculated dynamically as a percentage of the current buffer | ||
term_opts.size = get_dynamic_terminal_size(term_opts.direction, term_opts.size) |
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.
this needs to be a callback
term_opts.size = get_dynamic_terminal_size(term_opts.direction, term_opts.size) | |
term_opts.size = function() | |
get_dynamic_terminal_size(term_opts.direction, term_opts.size) | |
end |
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.
Are you sure? It doesn't work and it works without a callback
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.
@opalmay, it should be able to take a function
check their README for an example
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.
basically, it won't be dynamically calculated if it's not a function.
745e227
to
b0fc939
Compare
Co-authored-by: kylo252 <59826753+kylo252@users.noreply.github.com>
Co-authored-by: kylo252 <59826753+kylo252@users.noreply.github.com>
Co-authored-by: kylo252 <59826753+kylo252@users.noreply.github.com>
Co-authored-by: kylo252 <59826753+kylo252@users.noreply.github.com>
fb656b7
to
844e59f
Compare
lvim.builtin.terminal.terminals
eg:
lvim.builtin.terminal.terminals_defaults
is a table containing defaults fordirection
,horizontal_size
,vertical_size
that can be modified by the user.lvim.builtin.terminal.execs
is handled with soft deprecation and link to the docs docs: terminal keybinds lunarvim.org#367toggleterm:new
directly. One exception is size which is calculated as a percentage of the buffer size in the terminal direction (same as it was) andsize = 1
being a special case for a full screen terminal (like lazygit).desc
is being used for the keybind description, can be explicitly provided, otherwise it is based on the keybind cmd.lvim.builtin.terminal
from bracket to dot notation for intellisense.Depends on #3811