From c5ebfd4c4e1151c307633372cf3b3717f4f36208 Mon Sep 17 00:00:00 2001 From: Damian Senn Date: Tue, 3 Dec 2024 21:54:39 +0100 Subject: [PATCH 1/4] refactor: simplify github fetch and render errors inline --- lua/pipeline/config.lua | 2 + lua/pipeline/providers/github/rest/_api.lua | 146 +++++++++----------- lua/pipeline/providers/github/rest/init.lua | 44 +++--- lua/pipeline/store.lua | 2 + lua/pipeline/ui/render.lua | 9 ++ 5 files changed, 103 insertions(+), 100 deletions(-) diff --git a/lua/pipeline/config.lua b/lua/pipeline/config.lua index 1ab6229..ed351b3 100644 --- a/lua/pipeline/config.lua +++ b/lua/pipeline/config.lua @@ -40,6 +40,8 @@ local defaultConfig = { }, ---@class pipeline.config.Highlights highlights = { + ---@type vim.api.keyset.highlight + PipelineError = { link = 'LspDiagnosticsVirtualTextError' }, ---@type vim.api.keyset.highlight PipelineRunIconSuccess = { link = 'LspDiagnosticsVirtualTextHint' }, ---@type vim.api.keyset.highlight diff --git a/lua/pipeline/providers/github/rest/_api.lua b/lua/pipeline/providers/github/rest/_api.lua index 3d901eb..9cb204e 100644 --- a/lua/pipeline/providers/github/rest/_api.lua +++ b/lua/pipeline/providers/github/rest/_api.lua @@ -9,27 +9,26 @@ end local M = {} ---@class pipeline.providers.github.rest.FetchOptions ----@field method 'get'|'patch'|'post'|'put'|'delete' +---@field method? 'get'|'patch'|'post'|'put'|'delete' ---@field callback fun(err: plenary.curl.Error|nil, response: table|nil) ---@param server string ---@param path string ----@param opts? pipeline.providers.github.rest.FetchOptions -function M.fetch(server, path, opts) +---@param opts pipeline.providers.github.rest.FetchOptions +local function fetch(server, path, opts) if vim.fn.has('nvim-0.11') == 1 then vim.validate('server', server, 'string') vim.validate('path', path, 'string') - vim.validate('opts', opts, 'table', true) + vim.validate('opts', opts, 'table', 'opts must be a table') else vim.validate { server = { server, 'string' }, path = { path, 'string' }, - opts = { opts, 'table', true }, + opts = { opts, 'table' }, } end - opts = opts or {} - opts.callback = opts.callback and vim.schedule_wrap(opts.callback) + opts.callback = vim.schedule_wrap(opts.callback) local url = string.format('https://api.github.com%s', path) if server ~= 'github.com' then @@ -57,6 +56,38 @@ function M.fetch(server, path, opts) ) end +---@class pipeline.providers.github.rest.FetchJsonOptions: pipeline.providers.github.rest.FetchOptions +---@field map_response? fun(response: table): any + +---@param server string +---@param path string +---@param opts pipeline.providers.github.rest.FetchJsonOptions +local function fetch_json(server, path, opts) + return fetch( + server, + path, + vim.tbl_extend('force', opts, { + callback = function(err, response) + if err then + return opts.callback(err, nil) + end + + if not response then + return opts.callback({ message = 'No response body' }, nil) + end + + local response_data = vim.json.decode(response.body) + + if opts.map_response then + response_data = opts.map_response(response_data) + end + + opts.callback(nil, response_data) + end, + }) + ) +end + ---@class GhWorkflow ---@field id number ---@field node_id string @@ -75,29 +106,15 @@ end ---@param server string ---@param repo string ----@param opts? { callback?: fun(workflows: GhWorkflow[]): any } +---@param opts { callback: fun(error: plenary.curl.Error|nil, workflows: GhWorkflow[]): any } function M.get_workflows(server, repo, opts) - opts = opts or {} - - return M.fetch( + return fetch_json( server, string.format('/repos/%s/actions/workflows', repo), vim.tbl_deep_extend('force', opts, { - callback = function(err, response) - if err or not response then - return {} - end - - ---@type GhWorkflowsResponse | nil - local response_data = vim.json.decode(response.body) - - local ret = response_data and response_data.workflows or {} - - if opts.callback then - return opts.callback(ret) - else - return ret - end + ---@param response GhWorkflowsResponse + map_response = function(response) + return response.workflows end, }) ) @@ -121,43 +138,19 @@ end ---@field total_count number ---@field workflow_runs GhWorkflowRun[] ----@param opts? { callback?: fun(workflow_runs: GhWorkflowRun[]): any } -local function process_workflow_runs_response(opts) - opts = opts or {} - - ---@param err plenary.curl.Error | nil - ---@param response table - ---@return GhWorkflowRunsResponse - return function(err, response) - if err or not response then - return {} - end - - ---@type GhWorkflowRunsResponse | nil - local response_data = vim.json.decode(response.body) - - local ret = (response_data and response_data.workflow_runs or {}) - - if opts.callback then - return opts.callback(ret) - else - return ret - end - end -end - ---@param server string ---@param repo string ---@param per_page? integer ----@param opts? { callback?: fun(workflow_runs: GhWorkflowRun[]): any } +---@param opts { callback: fun(err: plenary.curl.Error|nil, workflow_runs: GhWorkflowRun[]): any } function M.get_repository_workflow_runs(server, repo, per_page, opts) - opts = opts or {} - - return M.fetch( + return fetch_json( server, string.format('/repos/%s/actions/runs', repo), vim.tbl_deep_extend('force', { query = { per_page = per_page } }, opts, { - callback = process_workflow_runs_response(opts), + ---@param response GhWorkflowRunsResponse + map_response = function(response) + return response.workflow_runs + end, }) ) end @@ -166,15 +159,16 @@ end ---@param repo string ---@param workflow_id integer|string ---@param per_page? integer ----@param opts? { callback?: fun(workflow_runs: GhWorkflowRun[]): any } +---@param opts { callback: fun(err: plenary.curl.Error|nil, workflow_runs: GhWorkflowRun[]): any } function M.get_workflow_runs(server, repo, workflow_id, per_page, opts) - opts = opts or {} - - return M.fetch( + return fetch_json( server, string.format('/repos/%s/actions/workflows/%d/runs', repo, workflow_id), vim.tbl_deep_extend('force', { query = { per_page = per_page } }, opts, { - callback = process_workflow_runs_response(opts), + ---@param response GhWorkflowRunsResponse + map_response = function(response) + return response.workflow_runs + end, }) ) end @@ -183,11 +177,9 @@ end ---@param repo string ---@param workflow_id integer|string ---@param ref string ----@param opts? table +---@param opts { body: table, callback?: fun(err: plenary.curl.Error|nil, response: unknown): any } function M.dispatch_workflow(server, repo, workflow_id, ref, opts) - opts = opts or {} - - return M.fetch( + return fetch( server, string.format( '/repos/%s/actions/workflows/%d/dispatches', @@ -227,29 +219,15 @@ end ---@param repo string ---@param workflow_run_id integer ---@param per_page? integer ----@param opts? { callback?: fun(workflow_runs: GhWorkflowRunJob[]): any } +---@param opts { callback: fun(err: plenary.curl.Error|nil, workflow_runs: GhWorkflowRunJob[]): any } function M.get_workflow_run_jobs(server, repo, workflow_run_id, per_page, opts) - opts = opts or {} - - return M.fetch( + return fetch_json( server, string.format('/repos/%s/actions/runs/%d/jobs', repo, workflow_run_id), vim.tbl_deep_extend('force', { query = { per_page = per_page } }, opts, { - callback = function(err, response) - if err or not response then - return {} - end - - ---@type GhWorkflowRunJobsResponse | nil - local response_data = vim.json.decode(response.body) - - local ret = response_data and response_data.jobs or {} - - if opts.callback then - return opts.callback(ret) - else - return ret - end + ---@param response GhWorkflowRunJobsResponse + map_response = function(response) + return response.jobs end, }) ) diff --git a/lua/pipeline/providers/github/rest/init.lua b/lua/pipeline/providers/github/rest/init.lua index de2825e..e252f50 100644 --- a/lua/pipeline/providers/github/rest/init.lua +++ b/lua/pipeline/providers/github/rest/init.lua @@ -61,17 +61,21 @@ function GithubRestProvider:fetch() local Mapper = require('pipeline.providers.github.rest._mapper') gh_api().get_workflows(self.server, self.repo, { - callback = function(workflows) + callback = function(err, workflows) self.store.update_state(function(state) - state.pipelines = vim.tbl_map(Mapper.to_pipeline, workflows) + state.error = err and err.message or nil + + if not state.error then + state.pipelines = vim.tbl_map(Mapper.to_pipeline, workflows) + end end) end, }) gh_api().get_repository_workflow_runs(self.server, self.repo, 100, { - callback = function(workflow_runs) + callback = function(err, workflow_runs) ---@type pipeline.Run[] - local runs = vim.tbl_map(Mapper.to_run, workflow_runs) + local runs = vim.tbl_map(Mapper.to_run, workflow_runs or {}) ---@type pipeline.Run[] local old_runs = vim .iter(vim.tbl_values(self.store.get_state().runs)) @@ -79,10 +83,14 @@ function GithubRestProvider:fetch() :totable() self.store.update_state(function(state) - state.latest_run = runs[1] - state.runs = utils.group_by(function(run) - return run.pipeline_id - end, runs) + state.error = err and err.message or nil + + if not state.error then + state.latest_run = runs[1] + state.runs = utils.group_by(function(run) + return run.pipeline_id + end, runs) + end end) local running_workflows = utils.uniq( @@ -96,14 +104,17 @@ function GithubRestProvider:fetch() for _, run in ipairs(running_workflows) do gh_api().get_workflow_run_jobs(self.server, self.repo, run.run_id, 20, { - callback = function(jobs) + callback = function(err, jobs) self.store.update_state(function(state) - state.jobs[run.run_id] = vim.tbl_map(Mapper.to_job, jobs) - - for _, job in ipairs(jobs) do - state.steps[job.id] = vim.tbl_map(function(step) - return Mapper.to_step(job.id, step) - end, job.steps) + state.error = err and err.message or nil + if not state.error then + state.jobs[run.run_id] = vim.tbl_map(Mapper.to_job, jobs) + + for _, job in ipairs(jobs) do + state.steps[job.id] = vim.tbl_map(function(step) + return Mapper.to_step(job.id, step) + end, job.steps) + end end end) end, @@ -170,12 +181,13 @@ function GithubRestProvider:dispatch(pipeline) pipeline.pipeline_id, 5, { - callback = function(workflow_runs) + callback = function(err, workflow_runs) local Mapper = require('pipeline.providers.github.rest._mapper') local runs = vim.tbl_map(Mapper.to_run, workflow_runs) store.update_state(function(state) + state.error = err and err.message or nil state.runs = utils.group_by( function(run) return run.pipeline_id diff --git a/lua/pipeline/store.lua b/lua/pipeline/store.lua index e7f4d59..a4d4ee5 100644 --- a/lua/pipeline/store.lua +++ b/lua/pipeline/store.lua @@ -8,6 +8,7 @@ local utils = require('pipeline.utils') ---@field title string ---@field repo string ---@field server string +---@field error string|nil ---@field pipelines pipeline.Pipeline[] ---@field runs table Runs indexed by pipeline id ---@field jobs table Jobs indexed by run id @@ -17,6 +18,7 @@ local initialState = { title = 'pipeline.nvim', repo = '', server = '', + error = nil, pipelines = {}, latest_run = nil, runs = {}, diff --git a/lua/pipeline/ui/render.lua b/lua/pipeline/ui/render.lua index 35e679c..2294713 100644 --- a/lua/pipeline/ui/render.lua +++ b/lua/pipeline/ui/render.lua @@ -77,6 +77,7 @@ function PipelineRender:render(bufnr) local state = self.store:get_state() self:title(state) + self:error(state) self:pipelines(state) self:trim() @@ -89,6 +90,14 @@ function PipelineRender:title(state) self:append(state.title):nl():nl() end +--- Render error message +---@param state pipeline.State +function PipelineRender:error(state) + if state.error then + self:append(state.error, 'PipelineError'):nl():nl() + end +end + --- Render each pipeline ---@param state pipeline.State function PipelineRender:pipelines(state) From 4ec6bc2285839f3d390d61515a8cbedf7ae8c22f Mon Sep 17 00:00:00 2001 From: Damian Senn Date: Wed, 4 Dec 2024 16:45:00 +0100 Subject: [PATCH 2/4] refactor: reduce nesting in github fetch --- lua/pipeline/providers/github/rest/init.lua | 51 ++++++++++++++------- 1 file changed, 35 insertions(+), 16 deletions(-) diff --git a/lua/pipeline/providers/github/rest/init.lua b/lua/pipeline/providers/github/rest/init.lua index e252f50..50f2362 100644 --- a/lua/pipeline/providers/github/rest/init.lua +++ b/lua/pipeline/providers/github/rest/init.lua @@ -58,6 +58,12 @@ end --TODO Maybe send lsp progress events when fetching, to interact -- with fidget.nvim function GithubRestProvider:fetch() + self:fetch_workflows() + self:fetch_runs() +end + +---@package +function GithubRestProvider:fetch_workflows() local Mapper = require('pipeline.providers.github.rest._mapper') gh_api().get_workflows(self.server, self.repo, { @@ -71,6 +77,11 @@ function GithubRestProvider:fetch() end) end, }) +end + +---@package +function GithubRestProvider:fetch_runs() + local Mapper = require('pipeline.providers.github.rest._mapper') gh_api().get_repository_workflow_runs(self.server, self.repo, 100, { callback = function(err, workflow_runs) @@ -103,27 +114,35 @@ function GithubRestProvider:fetch() ) for _, run in ipairs(running_workflows) do - gh_api().get_workflow_run_jobs(self.server, self.repo, run.run_id, 20, { - callback = function(err, jobs) - self.store.update_state(function(state) - state.error = err and err.message or nil - if not state.error then - state.jobs[run.run_id] = vim.tbl_map(Mapper.to_job, jobs) - - for _, job in ipairs(jobs) do - state.steps[job.id] = vim.tbl_map(function(step) - return Mapper.to_step(job.id, step) - end, job.steps) - end - end - end) - end, - }) + self:fetch_jobs(run.run_id) end end, }) end +---@param run_id number +---@package +function GithubRestProvider:fetch_jobs(run_id) + local Mapper = require('pipeline.providers.github.rest._mapper') + + gh_api().get_workflow_run_jobs(self.server, self.repo, run.run_id, 20, { + callback = function(err, jobs) + self.store.update_state(function(state) + state.error = err and err.message or nil + if not state.error then + state.jobs[run_id] = vim.tbl_map(Mapper.to_job, jobs) + + for _, job in ipairs(jobs) do + state.steps[job.id] = vim.tbl_map(function(step) + return Mapper.to_step(job.id, step) + end, job.steps) + end + end + end) + end, + }) +end + ---@param pipeline pipeline.providers.github.rest.Pipeline|nil function GithubRestProvider:dispatch(pipeline) if not pipeline then From ff3e5ac72e9bb602fc56b730ba355218555ea525 Mon Sep 17 00:00:00 2001 From: Damian Senn Date: Wed, 4 Dec 2024 16:46:18 +0100 Subject: [PATCH 3/4] refactor: reduce nesting in github fetch --- lua/pipeline/providers/github/rest/init.lua | 24 +++++++++++---------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/lua/pipeline/providers/github/rest/init.lua b/lua/pipeline/providers/github/rest/init.lua index 50f2362..d166c53 100644 --- a/lua/pipeline/providers/github/rest/init.lua +++ b/lua/pipeline/providers/github/rest/init.lua @@ -207,17 +207,19 @@ function GithubRestProvider:dispatch(pipeline) store.update_state(function(state) state.error = err and err.message or nil - state.runs = utils.group_by( - function(run) - return run.pipeline_id - end, - utils.uniq(function(run) - return run.run_id - end, { - unpack(runs), - unpack(vim.iter(state.runs):flatten():totable()), - }) - ) + if not state.error then + state.runs = utils.group_by( + function(run) + return run.pipeline_id + end, + utils.uniq(function(run) + return run.run_id + end, { + unpack(runs), + unpack(vim.iter(state.runs):flatten():totable()), + }) + ) + end end) end, } From a10210ed163e6fde6ff476f0c23fd49fc52e26b7 Mon Sep 17 00:00:00 2001 From: Damian Senn Date: Wed, 4 Dec 2024 16:50:03 +0100 Subject: [PATCH 4/4] fix: use of undefined var --- lua/pipeline/providers/github/rest/init.lua | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lua/pipeline/providers/github/rest/init.lua b/lua/pipeline/providers/github/rest/init.lua index d166c53..4b2b42b 100644 --- a/lua/pipeline/providers/github/rest/init.lua +++ b/lua/pipeline/providers/github/rest/init.lua @@ -125,7 +125,7 @@ end function GithubRestProvider:fetch_jobs(run_id) local Mapper = require('pipeline.providers.github.rest._mapper') - gh_api().get_workflow_run_jobs(self.server, self.repo, run.run_id, 20, { + gh_api().get_workflow_run_jobs(self.server, self.repo, run_id, 20, { callback = function(err, jobs) self.store.update_state(function(state) state.error = err and err.message or nil