diff --git a/CHANGELOG.md b/CHANGELOG.md index 2f46092..370682d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,3 +29,5 @@ Initial release. - Adaptive polling and a notification when a workflow run fails. - Gitea Actions CI running the test suites, script syntax checks, manifest validation, and a generated-image check. +- The sidebar keeps its open rows and scroll position across refreshes, and a + poll that changes nothing no longer redraws it at all. diff --git a/CLAUDE.md b/CLAUDE.md index a73c4c5..20aa5d7 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -70,8 +70,12 @@ Controllers mutate through it. It owns: - `repos`, `unmatchedHosts`, `instanceStatus` - per-run `jobs`/`artifacts`, per-PR `pullRequestDetail`, `secrets`, `variables` -- `onDidChange` → `main.js` debounces 150ms → every provider's `invalidate()` + - `reload()` +- `onDidChange` → `main.js` debounces 150ms, compares `store.signature()`, and + reloads only when it changed. An idle poll therefore leaves the tree, its + scroll position, and its open rows completely alone. Anything that changes + what is drawn has to appear in that signature — the branch filter is in there + for exactly that reason, and a live run contributes a coarse time bucket so + its elapsed clock keeps moving. - adaptive polling: `runningInterval` when anything is queued or running, else `idleInterval`. `refresh()` coalesces — a second call while one is in flight returns the same promise rather than stacking. @@ -81,6 +85,15 @@ only `roots()`. Nodes are plain objects (`views/nodes.js` `node()`), turned into `TreeItem`s centrally. A node with `load` fetches when expanded; the result is cached onto `element.children`. +It also owns **expansion state**, which is not optional bookkeeping. Nova +applies whatever `collapsibleState` `getTreeItem` returns, so a reload closes +every row the provider does not report as open. `main.js` feeds +`onDidExpandElement`/`onDidCollapseElement` into `setExpanded`, keyed by node +identifier. A node built with `expanded: true` seeds that set the first time its +identifier is seen, so a default-open section can still be closed and stay +closed. **Node identifiers must therefore be stable across rebuilds** — never +derive one from a counter or from anything that changes between refreshes. + **`controllers/selection.js`** — a command can arrive from a sidebar (Nova passes the `TreeView`), from a node directly, or from a menu with no argument at all. `selectedNode()` normalises all three; `resolveRepo()` falls back to the diff --git a/Scripts/main.js b/Scripts/main.js index 59ea9bb..7d02ae4 100644 --- a/Scripts/main.js +++ b/Scripts/main.js @@ -52,7 +52,10 @@ function buildSections(store) { return definitions.map((definition) => { const view = new TreeView(definition.id, { dataProvider: definition.provider }); nova.subscriptions.add(view); - return Object.assign(definition, { view: view }); + + const section = Object.assign(definition, { view: view }); + trackExpansion(section); + return section; }); } @@ -64,7 +67,25 @@ function reloadSections(sections) { } } -function registerCommands(store, refresh, sections, reviewComments) { +/** + * Mirrors expansion back into the provider. Nova applies whatever collapsible + * state getTreeItem returns, so a reload closes anything the provider does not + * know is open. + */ +function trackExpansion(section) { + const expand = section.view.onDidExpandElement((element) => + section.provider.setExpanded(element, true), + ); + const collapse = section.view.onDidCollapseElement((element) => + section.provider.setExpanded(element, false), + ); + + for (const subscription of [expand, collapse]) { + if (subscription) nova.subscriptions.add(subscription); + } +} + +function registerCommands(store, refresh, reviewComments, redraw) { auth.register(store, refresh); logs.register(store); artifacts.register(store); @@ -75,6 +96,9 @@ function registerCommands(store, refresh, sections, reviewComments) { browser.register(); nova.commands.register("gitea.refresh", async () => { + // An explicit refresh always redraws, even if nothing came back + // different, so the command visibly does something. + redraw(); await refresh({ rediscover: true }); await reviewComments.reload({ force: true }); }); @@ -129,10 +153,27 @@ exports.activate = function () { ); let timer = null; + let lastSignature = null; + + /** Drops the memo so the next change redraws unconditionally. */ + const redraw = () => { + lastSignature = null; + }; + const scheduleReload = () => { if (timer) clearTimeout(timer); timer = setTimeout(() => { timer = null; + + // Reloading rebuilds every row, which is disruptive even with + // expansion preserved, so a poll that changed nothing does nothing. + const signature = store.signature(); + if (signature === lastSignature) { + log.debug("nothing changed; leaving the sidebar alone"); + return; + } + lastSignature = signature; + reloadSections(sections); }, RELOAD_DEBOUNCE_MS); }; @@ -144,7 +185,7 @@ exports.activate = function () { log.error("refresh failed", String(error)); }); - registerCommands(store, refresh, sections, reviewComments); + registerCommands(store, refresh, reviewComments, redraw); watchPreferences(refresh); state = { diff --git a/Scripts/store.js b/Scripts/store.js index 22a4215..d25d8d1 100644 --- a/Scripts/store.js +++ b/Scripts/store.js @@ -328,6 +328,73 @@ class Store { this.notify(); } + /** + * Fingerprint of everything the sidebar draws. `main.js` compares it before + * reloading, so an idle poll that changes nothing leaves the tree — and the + * user's scroll position and open rows — completely alone. + */ + signature() { + const parts = []; + + // The loading flag only reaches the screen while there is nothing else + // to show; including it always would defeat the whole comparison. + if (!this.repos.length) parts.push(`loading:${this.loading}`); + parts.push(`error:${this.lastError || ""}`); + + for (const entry of this.unmatchedHosts) parts.push(`unmatched:${entry.host}`); + + // The branch filter changes what is drawn without changing any data. + const filter = config.branchFilter(); + parts.push(`filter:${filter.mode}:${filter.branch || ""}`); + + for (const [baseUrl, status] of this.instanceStatus) { + parts.push( + `instance:${baseUrl}:${status.ok}:${status.version || ""}:${status.user || ""}:${status.error || ""}`, + ); + } + + for (const repo of this.repos) { + parts.push(`repo:${repo.key}:${repo.branch || ""}`); + + const runs = this.runsFor(repo.key); + parts.push(`runs:${runs.error || ""}:${runs.legacy}`); + for (const run of runs.runs) { + parts.push(`run:${run.id}:${run.status}:${run.conclusion}:${run.updated_at || ""}`); + } + + const pulls = this.pullRequestsFor(repo.key); + parts.push(`pulls:${pulls.error || ""}`); + for (const pull of pulls.items) { + parts.push(`pull:${pull.number}:${pull.state}:${pull.draft}:${pull.updated_at}`); + } + + const secrets = this.secrets.get(repo.key); + if (secrets) { + parts.push( + `secrets:${secrets.error || ""}:${secrets.items.map((x) => x.name).join(",")}`, + ); + } + + const variables = this.variables.get(repo.key); + if (variables) { + parts.push( + `variables:${variables.error || ""}:${variables.items + .map((x) => `${x.name}=${x.data || x.value || ""}`) + .join(",")}`, + ); + } + } + + // While something is running its elapsed time is on screen and has to + // keep moving, so let the signature change once per active interval. + if (this.hasActiveRuns) { + const seconds = Math.max(5, config.runningInterval()); + parts.push(`tick:${Math.floor(Date.now() / 1000 / seconds)}`); + } + + return parts.join("|"); + } + // -- Housekeeping ------------------------------------------------------ _pruneCaches() { diff --git a/Scripts/views/builders.js b/Scripts/views/builders.js index 8473dea..ab44126 100644 --- a/Scripts/views/builders.js +++ b/Scripts/views/builders.js @@ -10,10 +10,10 @@ const { isActiveRun } = require("../store.js"); const { node, message } = nodes; -function stepNode(step, index) { +function stepNode(job, step, index) { const state = nodes.runState(step); return node("step", `${nodes.glyphFor(state)} ${step.name || `Step ${index + 1}`}`, { - identifier: `step-${step.number || index}`, + identifier: `step-${job.id}-${step.number || index}`, description: time.duration(step.started_at, step.completed_at), tooltip: `${step.name || ""}\nStatus: ${state}`, contextValue: "step", @@ -39,7 +39,9 @@ function jobNode(store, repo, run, job) { command: "gitea.viewJobLogs", repo: repo, data: { job: job, run: run, repoKey: repo.key }, - children: steps.length ? steps.map(stepNode) : null, + children: steps.length + ? steps.map((step, index) => stepNode(job, step, index)) + : null, }); } diff --git a/Scripts/views/nodes.js b/Scripts/views/nodes.js index d35469b..486172c 100644 --- a/Scripts/views/nodes.js +++ b/Scripts/views/nodes.js @@ -50,18 +50,17 @@ function reviewGlyph(state) { return "◆"; } -let counter = 0; - /** * Builds a tree node. `children` is a resolved array; `load` is an async * producer used for rows that fetch on expand. */ function node(kind, name, options = {}) { - counter += 1; const value = { kind: kind, name: name, - identifier: options.identifier || `${kind}-${counter}`, + // Identifiers must be stable across rebuilds: Nova matches them to + // work out which rows stay open when the tree reloads. + identifier: options.identifier || `${kind}:${name}`, descriptiveText: options.description || "", tooltip: options.tooltip || "", image: options.image || null, diff --git a/Scripts/views/provider.js b/Scripts/views/provider.js index aa8017b..d3f8b32 100644 --- a/Scripts/views/provider.js +++ b/Scripts/views/provider.js @@ -10,6 +10,35 @@ class NodeProvider { constructor(store) { this.store = store; this._roots = null; + + // Expansion has to be tracked here. A reload rebuilds every node, and + // the collapsible state we hand back is what Nova applies, so without + // this every refresh would close whatever the user had opened. + this._expanded = new Set(); + this._known = new Set(); + } + + /** Records what the user opened or closed, keyed by node identifier. */ + setExpanded(element, expanded) { + if (!element || !element.identifier) return; + if (expanded) this._expanded.add(element.identifier); + else this._expanded.delete(element.identifier); + } + + /** + * Whether a row should render open. A node marked `expanded` seeds the set + * the first time it is seen, so a default-open section can still be closed + * by hand and stay closed. + */ + isExpanded(element) { + const id = element.identifier; + if (!id) return element.expanded === true; + + if (!this._known.has(id)) { + this._known.add(id); + if (element.expanded) this._expanded.add(id); + } + return this._expanded.has(id); } /** Subclasses return an array of nodes (or a Promise of one). */ @@ -55,12 +84,12 @@ class NodeProvider { } getTreeItem(element) { - const collapsible = - element.children || element.load - ? element.expanded - ? TreeItemCollapsibleState.Expanded - : TreeItemCollapsibleState.Collapsed - : TreeItemCollapsibleState.None; + const expandable = Boolean(element.children || element.load); + const collapsible = !expandable + ? TreeItemCollapsibleState.None + : this.isExpanded(element) + ? TreeItemCollapsibleState.Expanded + : TreeItemCollapsibleState.Collapsed; const item = new TreeItem(element.name, collapsible); item.identifier = element.identifier; diff --git a/Tests/integration.test.js b/Tests/integration.test.js index 24207e4..5b3530f 100644 --- a/Tests/integration.test.js +++ b/Tests/integration.test.js @@ -242,6 +242,93 @@ function declaredCommands(manifest) { JSON.stringify(global.__messages), ); + t.section("the sidebar keeps its shape across refreshes"); + // A poll that rebuilds every row used to close whatever the user had open. + const workflowsView = views["gitea.workflows"]; + const workflowsProvider = workflowsView.dataProvider; + + const group = (await workflowsProvider.getChildren(null)).find( + (n) => n.kind === "workflow" && n.name === "ci.yml", + ); + const openMe = (await workflowsProvider.getChildren(group)).find((n) => n.data.run.id === 101); + + t.equals( + "a run starts collapsed", + workflowsProvider.getTreeItem(openMe).collapsibleState, + TreeItemCollapsibleState.Collapsed, + ); + + workflowsView.expand(group); + workflowsView.expand(openMe); + + // What a refresh does: throw the built tree away and rebuild it. + workflowsProvider.invalidate(); + const rebuiltGroup = (await workflowsProvider.getChildren(null)).find( + (n) => n.kind === "workflow" && n.name === "ci.yml", + ); + const rebuiltRun = (await workflowsProvider.getChildren(rebuiltGroup)).find( + (n) => n.data.run.id === 101, + ); + + t.check("the rebuilt run is a different object", rebuiltRun !== openMe); + t.equals("but carries the same identifier", rebuiltRun.identifier, openMe.identifier); + t.equals( + "and is still open", + workflowsProvider.getTreeItem(rebuiltRun).collapsibleState, + TreeItemCollapsibleState.Expanded, + ); + t.equals( + "its parent too", + workflowsProvider.getTreeItem(rebuiltGroup).collapsibleState, + TreeItemCollapsibleState.Expanded, + ); + + workflowsView.collapse(rebuiltRun); + workflowsProvider.invalidate(); + const closedAgain = (await workflowsProvider.getChildren( + (await workflowsProvider.getChildren(null)).find((n) => n.name === "ci.yml"), + )).find((n) => n.data.run.id === 101); + t.equals( + "closing it sticks too", + workflowsProvider.getTreeItem(closedAgain).collapsibleState, + TreeItemCollapsibleState.Collapsed, + ); + + // A section that opens by default must still be closable. + const branchProvider2 = views["gitea.branch"].dataProvider; + const folder = (await branchProvider2.getChildren(null)).find((n) => n.kind === "prFolder"); + t.equals( + "a default-open folder starts open", + branchProvider2.getTreeItem(folder).collapsibleState, + TreeItemCollapsibleState.Expanded, + ); + views["gitea.branch"].collapse(folder); + branchProvider2.invalidate(); + const folderAgain = (await branchProvider2.getChildren(null)).find((n) => n.kind === "prFolder"); + t.equals( + "and stays closed once the user closes it", + branchProvider2.getTreeItem(folderAgain).collapsibleState, + TreeItemCollapsibleState.Collapsed, + ); + + t.section("idle polls do not redraw"); + const first = store.signature(); + t.equals("an unchanged store yields an unchanged signature", store.signature(), first); + + const trackedRuns = store.runsFor(store.repos[0].key).runs; + const finished = trackedRuns.find((run) => run.id === 103); + const previousConclusion = finished.conclusion; + finished.conclusion = "success"; + t.check("a changed run conclusion changes it", store.signature() !== first); + finished.conclusion = previousConclusion; + + t.equals("and restoring it restores the signature", store.signature(), first); + + const branchFilterBefore = store.signature(); + nova.workspace.config.set("gitea.branchFilter", "all"); + t.check("changing the branch filter changes it", store.signature() !== branchFilterBefore); + nova.workspace.config.set("gitea.branchFilter", "current"); + t.section("api paths exercised"); console.log([...new Set(fake.calls.map((call) => call.split("?")[0]))].sort().join("\n")); diff --git a/Tests/nova-stub.js b/Tests/nova-stub.js index 62872e6..6c0e6c6 100644 --- a/Tests/nova-stub.js +++ b/Tests/nova-stub.js @@ -64,9 +64,17 @@ global.TreeView = class TreeView { this.dataProvider = options.dataProvider; this.visible = true; this.selection = []; + this.reloads = 0; + this._onExpand = []; + this._onCollapse = []; treeViews.push(this); } - reload() { return Promise.resolve(); } + reload() { this.reloads += 1; return Promise.resolve(); } + onDidExpandElement(cb) { this._onExpand.push(cb); return { dispose() {} }; } + onDidCollapseElement(cb) { this._onCollapse.push(cb); return { dispose() {} }; } + // Test helpers: pretend the user opened or closed a row. + expand(element) { for (const cb of this._onExpand) cb(element); } + collapse(element) { for (const cb of this._onCollapse) cb(element); } dispose() {} };