-
Notifications
You must be signed in to change notification settings - Fork 35
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 support for start_mode #175
Conversation
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.
Left some comments. Also make sure that as you're testing this you test different combinations of your starting mode + the start_mode
option. For example, try running vim.ui.input when you're already in visual mode and with start_mode = "insert"
. Try that for all the permutations to make sure that the start_mode
is respected in all cases and there's no weirdness.
lua/dressing/input.lua
Outdated
@@ -4,13 +4,25 @@ local patch = require("dressing.patch") | |||
local util = require("dressing.util") | |||
local M = {} | |||
|
|||
---@alias dressing.StartMode "insert" | "visual" | "normal" |
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.
Would be great to also provide support for select mode (see :help Select-mode
)
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.
Do you know a good way to get into select mode? The best option to me reading the help is CTRL-G
since it doesn't require changing any settings, but it's not working for me. It works if I do it manually (set start_mode
to visual and press CTRL-G
) but not using
vim.api.nvim_command("normal! v$")
local ctrl_g = vim.api.nvim_replace_termcodes("<C-g>", true, false, true)
vim.api.nvim_feedkeys(ctrl_g, "v", true)
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 seems to work for me
vim.cmd("normal! 0vg_")
vim.api.nvim_feedkeys(vim.api.nvim_replace_termcodes("<C-g>", true, false, true), "n", true)
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 snippet puts me in visual mode. Shouldn't it be "v"
, not "n"
? I thought it was a visual mode mapping.
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 still not sure what to do 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 just tried the snippet that I gave you on your branch and it works for me. Are you sure that it's not working? Is it possible that something else in your config is interfering with it?
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.
You're right. I just tried this, and I got into select mode.
nvim -u NORC --noplugin -c 'lua vim.cmd("normal! 0vg_")' -c 'lua vim.api.nvim_feedkeys(vim.api.nvim_replace_termcodes("<C-g>", true, false, true), "n", true)'
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.
Select mode is now working for me. What exactly is the advantage? Reading help, I see that enter deletes everything and goes into insert mode, but in our case enter confirms the input.
Thanks for the quick review. I addressed some of the comments. I left some to do because I'd like some feedback first. |
Only question you were waiting on was about select mode, right? |
Yes unless you also want to have a look at how I added back restoring the previous mode. |
lua/dressing/input.lua
Outdated
@@ -4,13 +4,25 @@ local patch = require("dressing.patch") | |||
local util = require("dressing.util") | |||
local M = {} | |||
|
|||
---@alias dressing.StartMode "insert" | "visual" | "normal" |
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 just tried the snippet that I gave you on your branch and it works for me. Are you sure that it's not working? Is it possible that something else in your config is interfering with it?
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 looks good. Once you add support for select mode and fix the CI, it's good to merge!
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.
Still some CI failures
I'm having some trouble with the tests. I saw some comments that |
Headless testing isn't great; as you've discovered there are some difficult inconsistencies to deal with. Unfortunately I don't have any better idea what hacks will help bypass this. Just try a few things, see if you can get something working, and if not no worries I won't block the PR for lack of tests. |
Since insert mode doesn't work well, I mocked |
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.
One nit and still some styling inconsistencies
Great job! Thanks for the PR! |
No, thank you! I really appreciate how patient you were with me. |
Implement
start_mode
. It's likestart_in_insert
, but more flexible.Context
Implement #174
Description
I added
start_mode
. I keptstart_in_insert
for backwards compatibility.Test Plan
I tried each value of
start_mode
with a default value and without. I also added automated tests of the backwards compatibility ofstart_in_insert
.