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

[Enhancement] Programatially Execute popup actions or switches without showing them #1446

Open
asmodeus812 opened this issue Aug 13, 2024 · 4 comments

Comments

@asmodeus812
Copy link
Contributor

It is sometimes nice to be able to use and trigger a specific action that a popup provides directly, for example open the branches poupup and trigger the c checkout action, programatically. As of now the solution is to call popup create and then feedkeys. This is somewhat cumbersome, it would be great the actions do not rely on a popup argument to be passed in by default, some actions (in branch actions for example) rely in some state from popup which makes it hard to reuse these actions outside of popups. Another option is to allow us to create popups without showing them or in other words - not calling the show method, (note, care should be appllied to avoid building any context related to showing on screen, such as creating a new buffer, lines etc, since it will never be shown, the poup object would simply act as a wrapper around the actions to be used only programatically, so in case any resources related to showing the popup are created outside the popup.show method, those should be pruned if possible), and expose an api that would allow us to trigger the actions from the popup api. Further more besides actions, one should be able to also programatically set the flags/switches.

@asmodeus812 asmodeus812 changed the title [Enhancement] Popups and actions should be more decoupled [Enhancement] Programatially Execute popup actions or switches without showing them Aug 13, 2024
@CKolkey
Copy link
Member

CKolkey commented Aug 13, 2024

I think this already exists, no? https://github.com/NeogitOrg/neogit/blob/master/doc/neogit.txt#L582-L607

Admittedly the popup state/context table passed into the constructor isn't optional, like you stipulate (and I think you're right that it should be).

@asmodeus812
Copy link
Contributor Author

asmodeus812 commented Aug 13, 2024

Yeah, for example, i just tried the cherry_pick with pick action, however even though there is a fallback to a commit picker i get the following issue - #popup.state.env.commits > 0 - while poup.sate.env is passed as an empty table in neogit.action, the rest are not.

local function get_commits(popup)
  local commits
  if #popup.state.env.commits > 0 then
    commits = popup.state.env.commits
  else
    commits = CommitSelectViewBuffer.new(
      git.log.list { "--max-count=256" },
      "Select one or more commits to cherry pick with <cr>, or <esc> to abort"
    ):open_async()
  end

  return commits or {}
end
E5108: Error executing lua: ...share/nvim/lazy/plenary.nvim/lua/plenary/async/async.lua:18: The coroutine failed with this message: ...rojects/neogit/lua/neogit/popups/cherry_pick/actions.lua
:11: attempt to get length of field 'commits' (a nil value)
stack traceback:
        [C]: in function 'error'
        ...share/nvim/lazy/plenary.nvim/lua/plenary/async/async.lua:18: in function 'callback_or_next'
        ...share/nvim/lazy/plenary.nvim/lua/plenary/async/async.lua:45: in function 'step'
        ...share/nvim/lazy/plenary.nvim/lua/plenary/async/async.lua:48: in function 'execute'
        ...share/nvim/lazy/plenary.nvim/lua/plenary/async/async.lua:118: in function <...share/nvim/lazy/plenary.nvim/lua/plenary/async/async.lua:117>
        /home/asmodeus/.projects/neogit/lua/neogit.lua:234: in function </home/asmodeus/.projects/neogit/lua/neogit.lua:206>
</home/asmodeus/.config/nvim/lua/user/command.lua:3187>

@CKolkey
Copy link
Member

CKolkey commented Aug 13, 2024

Ahh, yeah, thats just an oversight on my part. The Action API came way after the popup was built. There are probably a handful more like this.. probably the same conditional check too. I've got basically no free time right now to tackle this, but it should be pretty simple if you wanna have a go :)

@asmodeus812
Copy link
Contributor Author

Yeah i can check them out, had a quick look at all popups, most are verifying against existing properties on state.env, only the bisect popup is a bit complex does not seem to have any fallback logic at all.

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

No branches or pull requests

2 participants