[Feature Request]: Poll issue and pr buffers for changes

[ldelossa]

Feature Request

Hello,

I'd like to discuss a feature which I'd like to implement.

I often leave Octo.nvim in a terminal sitting around in the background.
It would be nice to get an indication on an open issue or PR buffer that something was updated.

I'm thinking as a first step, some non-intrusive method can be used to indicate the issue/pr has been updated.
My first thinking is, just place an asterisk somewhere in the buffer name, so attention is brought to either your tab bar displaying the buffer name or the status bar which shows the buffer name as well. Optionally, nvim.notify can be used, but this is only useful for individuals who look at their notifications in a list format.

After this first step, I think the idea of reloading the buffer with the new data can be explored.
There is already a reload function for this.
With reloads comes a bit of complication tho w/r/t pending comments.

There are a few problematic states here

1. User is currently editing a comment and reload occurs: naive fix: check for VIM mode, if in insert mode, defer the reload to next scheduled interval
2. User has a pending comment: in this case the user may be in normal mode so the above mitigation does not work. Instead, the Octo buffer should keep track of whether in a "pending comment" state and defer reloads until it's not.

Implementation

When the first (of possibly many) issue or pr buffers are opened an autocmd is triggered.
This autocmd will place a lua function, the poll function, on the internal event loop (vim.loop) at a configurable interval.

At each interval the poll function evaluates open issue/pr buffers, fetches the latest issue/pr datamodel from the api, and compares it with the local version.

If the issue/pr has changed, update the buffer name with an asterisk.

When the last issue/pr buffer is closed, the poller function acknowledges this and removes its reference from the event loop.

This entire feature is guarded by a config value and is opt-in.

A few points of conversation:

1. A single poll function can exist, which ranges over all open issue/pr, or multiple poll functions can exist for each issue/pr.
2. API requests for latest datamodels should be asyc (haven't looked at client but assume this is already the case).
3. API request, if graphql, can be limited to JUST returning the 'updated' field (don't know the name off-hand without looking at API), indicating the date and time of last modification.
4. Poll interval is configurable to tune for API rate limit friendliness.

Thoughts?

[wd60622]

Sounds like a good feature. I think that @GuillaumeLagrange had some ideas about this as well.

This might be helpful query:

local gh = require "octo.gh"

local query = [[
query($owner: String!, $name: String!, $number: Int!) {
  repository(owner: $owner, name: $name) {
    issueOrPullRequest(number: $number) {
      ... on Issue {
        updatedAt
      }
      ... on PullRequest {
        updatedAt
      }
    }
  }
}
]]

local fields = {
  owner = "pwntester",
  name = "octo.nvim",
  number = 815
}

local last_update_time = gh.api.graphql {
  query = query,
  fields = fields,
  jq = ".data.repository.issueOrPullRequest.updatedAt",
  opts = {
    mode = "sync",
  }
}

EDIT: It seems like it can be easiest done with issueOrPullRequest

[GuillaumeLagrange]

Hey @ldelossa, long time no see!

I think this goes hand in hand with the 1st and 2nd bullet point of #601, and would love to see this feature come to life.

The high level idea IMO would be to have a notion subscription to either an issue or a PR.

• Each subscription would be in charge of polling information in the background.
• An octo buffer would declare a subscription to a specific issue/PR
  • Subscriptions should be centralized to avoid polling multiple time information about the same PR/issue
• An octo buffer would have to handle being redrawn gracefully on data update -> we absolutely need to properly handle background source data update while the user is writing a comment for example (as you mentioned in the initial message)
• Octo could optionally start a subscription to the PR of the locally checked out branch
  • Side challenge: how to handle git branch change ?

The first step of this could be to have a simple polling mechanism linked with octo pr/issue buffers, and then we should expand on this and make it play nice with the notion of local PR, which I believe is a killer feature

[ldelossa]

Hey! @GuillaumeLagrange, indeed its been awhile, always appreciated your help with gh.nvim :-D.

I like what you're brewing here.
I want to take a small step toward this goal since we'll be doing this in our free time.

I am kinda thinking that tackling this:

An octo buffer would have to handle being redrawn gracefully on data update -> we absolutely need to properly handle background source data update while the user is writing a comment for example (as you mentioned in the initial message)

Can be done in a pretty isolated manor.
I.e. we can simply try this and test it out with Octo issue reload to make sure it works.

Do you have a solid idea of how you'd like to go about this. I don't have a entire world view of Octo.nvim right now, but my proposed solution would exist in basically tracking when a pending comment has been added to a Octo buffer. Maybe something as simple as adding a 'pending_comment' boolean field to the datastructures for the buffers? And then having the reload code evaluate this. I haven't checked tho, if something like this exists.

I'd be happy to chip away at the larger goal, by implementing this 'graceful reload' in a isolated PR.

[wd60622]

There is a "dirty" key for the buffers. Maybe the polling can be conditional of it not being dirty so that it doesnt conflict / override

[wd60622]

I use this command a lot while developing: #742

[ldelossa]

Sounds good! Just need to confirm dirty for only what we want to block reload for. For instance, if we just accidentally added a new line, maybe we shouldn't block reload on this?

[wd60622]

Sounds good! Just need to confirm dirty for only what we want to block reload for. For instance, if we just accidentally added a new line, maybe we shouldn't block reload on this?

Maybe good to make that assumption. Not sure if it is too strong. Maybe dirtySince can be added as well and we can make an assumption on time since change as opposed to content change