jackMort / ChatGPT.nvim

ChatGPT Neovim Plugin: Effortless Natural Language Generation with OpenAI's ChatGPT API
Apache License 2.0
3.77k stars 313 forks source link

add option to close popup in normal mode; default to <Esc> #139

Closed cmpadden closed 10 months ago

cmpadden commented 1 year ago

Requested feature in: https://github.com/jackMort/ChatGPT.nvim/issues/108

@jackMort can you please confirm that this change is also required in lua/chatgpt/flows/actions/init.lua?

Alternatively, we could allow <Esc> in the list of close bindings, but conditionally apply that to Normal mode only so that it does no mess with exiting Insert mode.

00sapo commented 1 year ago

Nui.nvim has its own method to close pop-ups. We should instead leave the user the ability to set its own key maps...

cmpadden commented 1 year ago

Nui.nvim has its own method to close pop-ups. We should instead leave the user the ability to set its own key maps...

@00sapo the user still has the ability to set their own mappings through the close and close_normal options. Or are you saying that nui.nvim has a way to globally set this keybinding for all pop-ups? FWIW, I was just following the existing implementation, but I'm happy to tweak it to a more standardized approach. Could you share an example or docs pertaining to this? I would be curious how it handles insert-mode and normal-mode bindings.

00sapo commented 1 year ago

I was meaning that in the config there should be a table like this (default empty):

{
 {'n', 'C-k', function ()
    print('do something')
  end, {silent=true}}
}

Then, whenever a Popup is created, we loop the table and pass each inner table to Popup:map (see docs. Similar for the input (I would use one single table for both input and popup).

cmpadden commented 1 year ago

@00sapo that's an interesting idea, but I'm not sure how I feel about the user configuring the callback function, nor do I know how we would want to handle that. For example, currently the callback contains:

chat_input.input_props.on_close()

Would we want the end-user to be aware that this is what is required? I do understand that this gives the user the most freedom in what is triggered on close, but at the cost of increased complexity.

Curious what you think too @jackMort

eyalk11 commented 1 year ago

I tried it :

Error executing Lua callback: ...\ekarni\.vim\plugged\ChatGPT.nvim/lua/chatgpt/module.lua:195: attempt to index field 'keymaps' (a nil value)
stack traceback:
    ...\ekarni\.vim\plugged\ChatGPT.nvim/lua/chatgpt/module.lua:195: in function 'open_chat'
    ...\ekarni\.vim\plugged\ChatGPT.nvim/lua/chatgpt/module.lua:289: in function 'openChat'
    C:\Users\ekarni\.vim\plugged\ChatGPT.nvim/lua/chatgpt.lua:25: in function 'openChat'
    ...sers\ekarni\.vim\plugged\ChatGPT.nvim\plugin\chatgpt.lua:2: in function <...sers\ekarni\.vim\plugged\ChatGPT.nvim\plugin\chatgpt.lua:1>
11 more lines

is it just me? I do setup()

cmpadden commented 1 year ago

I tried it :

Error executing Lua callback: ...\ekarni\.vim\plugged\ChatGPT.nvim/lua/chatgpt/module.lua:195: attempt to index field 'keymaps' (a nil value)
stack traceback:
  ...\ekarni\.vim\plugged\ChatGPT.nvim/lua/chatgpt/module.lua:195: in function 'open_chat'
  ...\ekarni\.vim\plugged\ChatGPT.nvim/lua/chatgpt/module.lua:289: in function 'openChat'
  C:\Users\ekarni\.vim\plugged\ChatGPT.nvim/lua/chatgpt.lua:25: in function 'openChat'
  ...sers\ekarni\.vim\plugged\ChatGPT.nvim\plugin\chatgpt.lua:2: in function <...sers\ekarni\.vim\plugged\ChatGPT.nvim\plugin\chatgpt.lua:1>
11 more lines

is it just me? I do setup()

@eyalk11 I have resolved this issue in https://github.com/jackMort/ChatGPT.nvim/pull/139/commits/d57ba6dbdf6d69d959a5e074f28098c8ded58338.

This was because config keymaps has become chat.keymaps since this original PR was created. It should now work for you.

jackMort commented 10 months ago

sorry for delay, could you please resolve conflicts, if you still using this plugin :)

cmpadden commented 10 months ago

sorry for delay, could you please resolve conflicts, if you still using this plugin :)

Hi @jackMort - it looks like the codebase has changed quite a bit since this PR was opened. The implementation here is no longer applicable, so I'm going to close this for now, thank you for reviewing.