feat: fzf-lua picker, phase 2

[milogert]

Describe what this PR does / why we need it

TODOS

• [x] Remove logging
• [x] Implement gists
• [x] Implement issue templates
• [x] Debug error in repos
• [ ] Slowness in the search picker
• [x] LINT

This PR implements the remaining pickers for fzf-lua (started in https://github.com/pwntester/octo.nvim/pull/396).

• Actions
• Assigned labels
• Assignees
• Gists
• Issue templates
• Labels
• Pending threads
• Repos
• Review commits
• Search
• Users

Does this pull request fix one issue?

Fixes #206

Describe how you did it

Same as the previous PR, I followed similar patterns. There are a few updates here to the original pickers. As I learned more about fzf-lua I found better ways to make the UI more responsive by utilizing callbacks to populate the picker rather than waiting on the gh response.

I've also moved away from the formatted_* tables that some pickers use since the data required can just be held in a single line and masked. For instance, when selecting a user the line behind the scenes looks like <id> <login> but the UI just shows <login>. When trying to actually pass data back to GitHub we can split the line and grab just the <id> to pass back. This removes the need for populating data structures or building out tables to use later.

Describe how to verify it

1. Have this branch installed as well as fzf-lua.
2. Test out following pickers:
   • Actions
   • Assigned labels
   • Assignees
   • Gists
   • Issue templates
   • Labels
   • Pending threads
   • Repos
   • Review commits
   • Search
   • Users

Special notes for reviews

[milogert]

@pwntester I am seeing an error when using OctoBuffer:configure in the review_thread and repo previewers.

@ibhagwan and I narrowed it down to this line https://github.com/milogert/octo.nvim/blob/5b9d406fe328b19329bb0f23b3111b9c6c576751/lua/octo/model/octo-buffer.lua#L241-L241

Could you help us understand what exactly the OctoBuffer:configure method is doing? A visible thing I saw is that when I don't call configure the README that is loaded my repo previewer doesn't get highlighted but I'm positive it's doing more than that to help Octo do it's thing.

This may very well not be necessary for the fzf-lua picker but I just wanted to make sure to cover our bases here

[pwntester]

---Confgiures the buffer
function OctoBuffer:configure()
  -- configure buffer
  vim.api.nvim_buf_call(self.bufnr, function()
    local use_signcolumn = config.get_config().ui.use_signcolumn
    vim.cmd [[setlocal filetype=octo]]
    vim.cmd [[setlocal buftype=acwrite]]
    vim.cmd [[setlocal omnifunc=v:lua.octo_omnifunc]]
    vim.cmd [[setlocal conceallevel=2]]
    vim.cmd [[setlocal nonumber norelativenumber nocursorline wrap]]
    if use_signcolumn then
      vim.cmd [[setlocal signcolumn=yes]]
      autocmds.update_signcolumn(self.bufnr)
    end
  end)

  self:apply_mappings()
end

It just configures an Octo buffer:

• Sets the filetype to octo
• Specifies that the buffer is not connected to a file and that it will be written using an autocommand
• Sets the completion function
• Sets the conceal level and other options
• Depending on your configuration, it will show or hide the sign column.

As for the setlocal buftype=acwrite line. When writing (saving) an Octo buffer, the BufWriteCmd autocommand will call save_buffer to sync it with GitHub.

What is the specific error you are getting?

[ibhagwan]

What is the specific error you are getting?

For some reason, setting acwrite will fail the call to nvim_buf_set_win with the rather opaque nvim error Failed to set buffer 12 (https://github.com/ibhagwan/fzf-lua/issues/759#issuecomment-1574262951).

As for the setlocal buftype=acwrite line. When writing (saving) an Octo buffer, the BufWriteCmd autocommand will call save_buffer to sync it with GitHub.

This is interesting and perhaps the reason for the failure, the set win call is part of a function which sets the buffer without triggering an autocmd (to prevent LSPs from starting due to fzf-lua previews and other potential issues):

https://github.com/ibhagwan/fzf-lua/blob/3c260cce2499abe161e222fa1d69f3a7602a243e/lua/fzf-lua/utils.lua#L671-L676

function M.win_set_buf_noautocmd(win, buf)
  local save_ei = vim.o.eventignore
  vim.o.eventignore = "all"
  vim.api.nvim_win_set_buf(win, buf)
  vim.o.eventignore = save_ei
end

@milogert, perhaps in Octo’s previewers we can call vim.api.nvim_win_set_buf directly? I have a feeling the failure is due to the conflict of acwrite expecting an autocmd trigger which doesn’t happen due to eventignore=all.

[milogert]

As for the setlocal buftype=acwrite line. When writing (saving) an Octo buffer, the BufWriteCmd autocommand will call save_buffer to sync it with GitHub.

So.. For fzf-lua, I think (at least I'm pretty sure) you can't edit the preview buffer at all. So setting anything to do with writing things out would be useless in this case, since you aren't doing any edits. @ibhagwan can correct me if I'm wrong here. I might just be missing a really useful feature of the previewer

I'm wrong here! Pay no attention to ☝️

@milogert, perhaps in Octo’s previewers we can call vim.api.nvim_win_set_buf directly? I have a feeling the failure is due to the conflict of acwrite expecting an autocmd trigger which doesn’t happen due to eventignore=all.

Yes, that works! I copied most of the set_preview_buf method over and replaced the utils call with a direct call to vim.api.nvim_win_set_buf

[ibhagwan]

So.. For fzf-lua, I think (at least I'm pretty sure) you can't edit the preview buffer at all.

I’m not sure why you came to this conclusion, there shouldn’t be any limitations to an fzf-lua preview buffer, it’s just a buffer.

[milogert]

Alright so after messing with this again I realized I still had the acwrite line commented out. Even when using the raw vim.api.nvim_win_set_buf it still doesn't work (same error).

[milogert]

Ok, after experimenting more I think I got it. After looking more closely I saw that the OctoBuffer has a method named render_repo which calls writers.write_repo but also sets the modified flag of the buffer to false. This was the main thing I needed to do. I also added this to the OctoBuffer:render_threads method: https://github.com/milogert/octo.nvim/blob/65f38a3de97a2f053a3afff1ccbd845ead844a0a/lua/octo/model/octo-buffer.lua#L232-L232.

That fixed the error for a bit until I uncommented the README rendering portion of writers.write_repo. Since that's async I needed to add the modified flag reset there too.

I believe that fixes the error we were seeing!

For more details, it seems like the nvim_buf_set_lines function was causing problems. When I got rid of calls to that in the writers module things started working, in the sense that the error did show itself.

[milogert]

@pwntester @ibhagwan with the exception of the :Octo search slowness, I think this is ready for review. I believe I have finished the base implementation of every other picker

[pwntester]

Thanks a lot @milogert, that was an impressive work! You should by now have a good understanding of how Octo works so let me know if you want to be a collaborator of this project, it would be nice to have you onboard!

[milogert]

Thanks for the reviews! I'd love to be a collaborator, we can chat about what that means in terms of responsibilities outside of this PR :)

So relating to this: I was doing another pass on this to make sure everything worked well (after updating my neovim config from the latest on NixOS). I found that these lines https://github.com/pwntester/octo.nvim/blob/f498fd88bc0d9983a7fb566fa5535f8e38b874c0/lua/octo/gh/init.lua#L53-L59 caused me some issues. After switching the stderr_result to be result my issue was fixed. Was this an oversight somewhere along the line or am I just missing something?

The problem I saw was that the stderr_result didn't contain the proper data since it was written to stdout (i.e. when I run the gh command on my console it works normally).

[pwntester]

Did you also upgrade gh? perhaps they changed the file descriptor they are sending the output of gh auth status to.

With my current version (gh version 2.29.0 (2023-05-10)), gh auth status 1> /tmp/out returns nothing but gh auth status 2> /tmp/err returns the expected data

[pwntester]

Ok, after updating to gh version 2.31.0 (2023-06-20) I get the same results as you do (output to stdout instead of stderr). I will change the code to try both options to support all versions

[pwntester]

Fixed the gh issue in https://github.com/pwntester/octo.nvim/commit/22328c578bc013fa4b0cef3d00af35efe0c0f256