diff --git a/CHANGELOG.md b/CHANGELOG.md index e3eecced..c1285508 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -46,6 +46,14 @@ heading, and (b) bumping `pubspec.yaml` `version:` in the same commit. ### Fixed +- The editor pane now opens over the Claude pane when a file is opened — the + reader's edit pencil, a file-tree click, or a decision's edit all reveal the + editor tab now (it was contributed but never activated). (T-197) +- Clicking a decision opens it on the first click. The right-pane readers + (markdown + decisions) now share a retained back/forward nav history that + survives the tab switch, so the selection that reveals a reader is no longer + lost before the widget subscribes; back/forward re-emit through that history. + (T-196) - The markdown reader can now open user-scope Claude config files (skills / agents / commands under `~/.claude`), not just repo-local ones. `files.read` gained a read allow-list covering the workspace plus the trusted Claude config diff --git a/lib/builtin/decisions/src/decision_detail_view.dart b/lib/builtin/decisions/src/decision_detail_view.dart index 73de1fcf..76c04f89 100644 --- a/lib/builtin/decisions/src/decision_detail_view.dart +++ b/lib/builtin/decisions/src/decision_detail_view.dart @@ -14,32 +14,43 @@ class DecisionDetailView extends StatefulWidget { State createState() => _DecisionDetailViewState(); } -class _DecisionDetailViewState extends State with ReaderHistoryMixin { +class _DecisionDetailViewState extends State { Map? _decision; bool _loading = false; StreamSubscription? _sub; + ReaderNav? _nav; @override void didChangeDependencies() { super.didChangeDependencies(); if (_sub != null) return; final kernel = ClideKernel.of(context); - _sub = kernel.messages.subscribe(publisher: 'builtin.decisions', channel: 'selection').listen((msg) { + // The back/forward history is the retained right-pane nav (T-196): + // it records selections (even before this view mounts) and re-emits + // them on the single 'load' channel this view loads from. + _nav = kernel.readerNav.navFor('builtin.decisions', dataKey: 'id')..addListener(_onNavChanged); + _sub = kernel.messages.subscribe(publisher: 'builtin.decisions', channel: 'load').listen((msg) { final id = msg.data['id'] as String?; if (id != null) unawaited(_load(id)); }); - if (widget.initialId != null) { - unawaited(_load(widget.initialId!)); - } + // Grab the latest entry the nav already holds — the selection that + // revealed this tab arrived before we could subscribe to 'load'. + final current = _nav!.current ?? widget.initialId; + if (current != null) unawaited(_load(current)); + } + + void _onNavChanged() { + if (mounted) setState(() {}); // refresh action-bar button state (pin, etc.) } @override void dispose() { _sub?.cancel(); + _nav?.removeListener(_onNavChanged); super.dispose(); } - /// Load [id] and push it onto the history stack (external navigation). + /// Fetch + display [id]. History lives in [ReaderNav]; this never pushes. Future _load(String id) async { setState(() => _loading = true); final kernel = ClideKernel.of(context); @@ -52,42 +63,12 @@ class _DecisionDetailViewState extends State with ReaderHist _loading = false; _decision = resp.ok ? resp.data : null; }); - if (resp.ok) historyPush(id); } - /// Load [id] WITHOUT pushing onto the history stack (back/forward nav). - Future _loadInPlace(String id) async { - setState(() => _loading = true); - final kernel = ClideKernel.of(context); - final resp = await kernel.ipc.request('pql.decisions.read', args: {'id': id}); - if (!mounted) return; - if (resp.ok) { - kernel.messages.publish('builtin.decisions', 'focus', {'id': id}); - } - setState(() { - _loading = false; - _decision = resp.ok ? resp.data : null; - }); - } - - void _onBack() { - final entry = historyBack(); - if (entry != null) _loadInPlace(entry); - } - - void _onForward() { - final entry = historyForward(); - if (entry != null) _loadInPlace(entry); - } - - void _onPin() { - pinCurrent(); - } - - void _onJumpToPin() { - final entry = jumpToPin(); - if (entry != null) _loadInPlace(entry); - } + void _onBack() => _nav?.back(); + void _onForward() => _nav?.forward(); + void _onPin() => _nav?.pin(); + void _onJumpToPin() => _nav?.jumpToPin(); void _onEdit() { final filePath = _decision?['file_path'] as String?; @@ -131,13 +112,13 @@ class _DecisionDetailViewState extends State with ReaderHist subtitle: title, trailing: [ ReaderActionBar( - canGoBack: canGoBack, - canGoForward: canGoForward, - hasPinned: hasPinned, - onBack: canGoBack ? _onBack : null, - onForward: canGoForward ? _onForward : null, + canGoBack: _nav?.canGoBack ?? false, + canGoForward: _nav?.canGoForward ?? false, + hasPinned: _nav?.hasPinned ?? false, + onBack: (_nav?.canGoBack ?? false) ? _onBack : null, + onForward: (_nav?.canGoForward ?? false) ? _onForward : null, onPin: _decision != null ? _onPin : null, - onJumpToPin: hasPinned ? _onJumpToPin : null, + onJumpToPin: (_nav?.hasPinned ?? false) ? _onJumpToPin : null, onEdit: filePath != null ? _onEdit : null, ), ], diff --git a/lib/builtin/decisions/src/extension.dart b/lib/builtin/decisions/src/extension.dart index 3cf9ea81..d3c20ba2 100644 --- a/lib/builtin/decisions/src/extension.dart +++ b/lib/builtin/decisions/src/extension.dart @@ -20,6 +20,10 @@ class DecisionsExtension extends ClideExtension { @override Future activate(ClideExtensionContext ctx) async { + // Ensure the retained nav exists so it records selections + emits + // loads whether or not the reader is mounted (T-196). This handler + // only reveals the tab; the nav owns load + history. + ctx.readerNav.navFor(id, dataKey: 'id'); _sub = ctx.messages.subscribe(publisher: id, channel: 'selection').listen((msg) { if (msg.data['id'] is! String) return; ctx.arrangement.setVisible(Slots.contextPanel, true); diff --git a/lib/builtin/editor/src/extension.dart b/lib/builtin/editor/src/extension.dart index b54e10a3..35b6f6f1 100644 --- a/lib/builtin/editor/src/extension.dart +++ b/lib/builtin/editor/src/extension.dart @@ -1,3 +1,5 @@ +import 'dart:async'; + import 'package:clide/builtin/editor/src/editor_view.dart'; import 'package:clide/extension/extension.dart'; import 'package:clide/kernel/kernel.dart'; @@ -15,6 +17,25 @@ class EditorExtension extends ClideExtension { @override List get dependsOn => const []; + StreamSubscription? _sub; + + /// Reveal the editor tab when a buffer opens or becomes active. + /// `editor.open` opens the buffer daemon-side and emits the event, + /// but nothing else brings the workspace tab to front — without this + /// the editor never appears over the Claude pane (T-197). The view's + /// `hydrate()` pulls the active buffer once it mounts. + @override + Future activate(ClideExtensionContext ctx) async { + _sub = ctx.events.on().listen((e) { + if (e.subsystem != 'editor') return; + if (e.kind != 'editor.opened' && e.kind != 'editor.active-changed') return; + ctx.panels.activateTab(Slots.workspace, 'editor.active'); + }); + } + + @override + Future deactivate() async => _sub?.cancel(); + @override List get contributions => [ TabContribution( diff --git a/lib/builtin/markdown/src/extension.dart b/lib/builtin/markdown/src/extension.dart index 74ea568c..cc24dcf0 100644 --- a/lib/builtin/markdown/src/extension.dart +++ b/lib/builtin/markdown/src/extension.dart @@ -4,7 +4,6 @@ import 'package:clide/builtin/markdown/src/markdown_viewer.dart'; import 'package:clide/extension/extension.dart'; import 'package:clide/kernel/kernel.dart'; import 'package:clide/widgets/widgets.dart'; -import 'package:flutter/widgets.dart'; class MarkdownExtension extends ClideExtension { @override @@ -31,13 +30,13 @@ class MarkdownExtension extends ClideExtension { @override Future activate(ClideExtensionContext ctx) async { + // Ensure the retained nav exists so it records selections + emits + // loads whether or not the viewer is mounted (T-196). This handler + // only reveals the tab; the nav owns load + history. + ctx.readerNav.navFor(id, dataKey: 'path'); _sub = ctx.messages.subscribe(publisher: id, channel: 'selection').listen((msg) { - final path = msg.data['path'] as String?; - if (path == null) return; + if (msg.data['path'] is! String) return; ctx.panels.activateTab(Slots.contextPanel, 'markdown.viewer'); - WidgetsBinding.instance.addPostFrameCallback((_) { - ctx.messages.publish(id, 'load', {'path': path}); - }); }); } diff --git a/lib/builtin/markdown/src/markdown_viewer.dart b/lib/builtin/markdown/src/markdown_viewer.dart index c2fc8fe3..6e773548 100644 --- a/lib/builtin/markdown/src/markdown_viewer.dart +++ b/lib/builtin/markdown/src/markdown_viewer.dart @@ -12,30 +12,43 @@ class MarkdownViewer extends StatefulWidget { State createState() => _MarkdownViewerState(); } -class _MarkdownViewerState extends State with ReaderHistoryMixin { +class _MarkdownViewerState extends State { String? _path; String? _content; String? _error; StreamSubscription? _selectionSub; + ReaderNav? _nav; @override void didChangeDependencies() { super.didChangeDependencies(); if (_selectionSub != null) return; final kernel = ClideKernel.of(context); + // Back/forward history is the retained right-pane nav (T-196); it + // records selections and re-emits them on the 'load' channel. + _nav = kernel.readerNav.navFor('builtin.markdown', dataKey: 'path')..addListener(_onNavChanged); _selectionSub = kernel.messages.subscribe(publisher: 'builtin.markdown', channel: 'load').listen((msg) { final path = msg.data['path'] as String?; if (path != null) _loadFile(path); }); + // Grab the latest entry the nav already holds (a selection that + // revealed this tab before we subscribed). + final current = _nav!.current; + if (current != null) _loadFile(current); + } + + void _onNavChanged() { + if (mounted) setState(() {}); // refresh action-bar button state } @override void dispose() { _selectionSub?.cancel(); + _nav?.removeListener(_onNavChanged); super.dispose(); } - /// Load [path] and push it onto the history stack (external navigation). + /// Fetch + display [path]. History lives in [ReaderNav]; never pushes. Future _loadFile(String path) async { final kernel = ClideKernel.of(context); final resp = await kernel.ipc.request('files.read', args: {'path': path}); @@ -47,50 +60,15 @@ class _MarkdownViewerState extends State with ReaderHistoryMixin _content = resp.data['content'] as String? ?? ''; _error = null; }); - historyPush(path); } else { setState(() => _error = resp.error?.message); } } - /// Load [path] WITHOUT pushing onto the history stack (back/forward nav). - Future _loadFileInPlace(String path) async { - final kernel = ClideKernel.of(context); - final resp = await kernel.ipc.request('files.read', args: {'path': path}); - if (!mounted) return; - if (resp.ok) { - kernel.messages.publish('builtin.markdown', 'focus', {'path': path}); - setState(() { - _path = path; - _content = resp.data['content'] as String? ?? ''; - _error = null; - }); - } else { - setState(() => _error = resp.error?.message); - } - } - - void _onBack() { - final entry = historyBack(); - if (entry != null) _loadFileInPlace(entry); - } - - void _onForward() { - final entry = historyForward(); - if (entry != null) _loadFileInPlace(entry); - } - - void _onPin() { - pinCurrent(); - } - - void _onJumpToPin() { - final entry = jumpToPin(); - if (entry != null) { - // Navigate directly without re-pushing (pin jump is a quick-return). - _loadFileInPlace(entry); - } - } + void _onBack() => _nav?.back(); + void _onForward() => _nav?.forward(); + void _onPin() => _nav?.pin(); + void _onJumpToPin() => _nav?.jumpToPin(); void _onEdit() { final p = _path; @@ -127,13 +105,13 @@ class _MarkdownViewerState extends State with ReaderHistoryMixin subtitle: '${_content!.split('\n').length} lines', trailing: [ ReaderActionBar( - canGoBack: canGoBack, - canGoForward: canGoForward, - hasPinned: hasPinned, - onBack: canGoBack ? _onBack : null, - onForward: canGoForward ? _onForward : null, + canGoBack: _nav?.canGoBack ?? false, + canGoForward: _nav?.canGoForward ?? false, + hasPinned: _nav?.hasPinned ?? false, + onBack: (_nav?.canGoBack ?? false) ? _onBack : null, + onForward: (_nav?.canGoForward ?? false) ? _onForward : null, onPin: _path != null ? _onPin : null, - onJumpToPin: hasPinned ? _onJumpToPin : null, + onJumpToPin: (_nav?.hasPinned ?? false) ? _onJumpToPin : null, onEdit: _path != null ? _onEdit : null, ), ], diff --git a/lib/builtin/shared/reader_chrome.dart b/lib/builtin/shared/reader_chrome.dart index 323e86b3..08b8e244 100644 --- a/lib/builtin/shared/reader_chrome.dart +++ b/lib/builtin/shared/reader_chrome.dart @@ -1,118 +1,15 @@ /// Shared action-bar chrome for sidebar reader widgets (T-189, T-190, T-191). /// -/// Provides: -/// - [ReaderHistory] — back/forward history stack with browser semantics. -/// - [ReaderActionBar] — the visible action bar widget: back, forward, pin, -/// edit pencil. Plugs into a [ClidePaneChrome] via its `trailing:` slot. -/// -/// Usage: mix [ReaderHistoryMixin] into a [State] to get back/forward/pin state -/// management, then put a [ReaderActionBar] in [ClidePaneChrome.trailing]. +/// Provides [ReaderActionBar] — the visible action bar: back, forward, pin, +/// edit pencil. Plugs into a [ClidePaneChrome] via its `trailing:` slot. The +/// retained back/forward history that drives it is the kernel `ReaderNav` +/// (one per right-pane reader); this file is just the buttons. library; import 'package:clide/kernel/kernel.dart'; import 'package:clide/widgets/widgets.dart'; import 'package:flutter/widgets.dart'; -// --------------------------------------------------------------------------- -// History model -// --------------------------------------------------------------------------- - -/// Back/forward history stack. [T] is the entry type (String path for -/// markdown, String id for decisions). -/// -/// Standard browser semantics: -/// - [push] appends at [_index+1] and truncates any forward entries. -/// - [back]/[forward] adjust the index without re-pushing. -/// - [canGoBack]/[canGoForward] drive the enabled state of the buttons. -class ReaderHistory { - final List _stack = []; - int _index = -1; - - bool get canGoBack => _index > 0; - bool get canGoForward => _index < _stack.length - 1; - - T? get current => _index >= 0 && _index < _stack.length ? _stack[_index] : null; - - /// Push a new entry, truncating any forward history. - void push(T entry) { - if (_index >= 0 && _stack[_index] == entry) { - // Same entry as current — don't push a duplicate. - return; - } - // Truncate forward history. - if (_index < _stack.length - 1) { - _stack.removeRange(_index + 1, _stack.length); - } - _stack.add(entry); - _index = _stack.length - 1; - } - - /// Move back one step. Returns the entry now current, or null. - T? back() { - if (!canGoBack) return null; - _index--; - return _stack[_index]; - } - - /// Move forward one step. Returns the entry now current, or null. - T? forward() { - if (!canGoForward) return null; - _index++; - return _stack[_index]; - } -} - -// --------------------------------------------------------------------------- -// Mixin -// --------------------------------------------------------------------------- - -/// Mix into a [State] that owns a [ReaderHistory] and an optional pin. -/// -/// The concrete state must call [historyPush] whenever it loads a new -/// entry (NOT when navigating back/forward — those call [historyBack] / -/// [historyForward] and then load the returned entry without re-pushing). -mixin ReaderHistoryMixin on State { - final ReaderHistory _history = ReaderHistory(); - T? _pinned; - - bool get canGoBack => _history.canGoBack; - bool get canGoForward => _history.canGoForward; - bool get hasPinned => _pinned != null; - T? get pinnedEntry => _pinned; - - /// Record that the reader is now showing [entry]. Must be called AFTER - /// setState has been applied so the action-bar buttons rebuild. - void historyPush(T entry) { - setState(() => _history.push(entry)); - } - - /// Navigate back. Returns the entry to load, or null if already at start. - T? historyBack() { - final entry = _history.back(); - if (entry != null) setState(() {}); - return entry; - } - - /// Navigate forward. Returns the entry to load, or null if at end. - T? historyForward() { - final entry = _history.forward(); - if (entry != null) setState(() {}); - return entry; - } - - /// Set or replace the pin with the current entry. - void pinCurrent() { - final cur = _history.current; - if (cur == null) return; - setState(() => _pinned = cur); - } - - /// Returns the pinned entry, or null if none set. - T? jumpToPin() { - return _pinned; - } -} - // --------------------------------------------------------------------------- // Action bar widget // --------------------------------------------------------------------------- diff --git a/lib/extension/src/extension.dart b/lib/extension/src/extension.dart index bc8ba7bf..7cb8828a 100644 --- a/lib/extension/src/extension.dart +++ b/lib/extension/src/extension.dart @@ -2,6 +2,7 @@ import 'package:clide/extension/src/contribution.dart'; import 'package:clide/kernel/src/clipboard.dart'; import 'package:clide/kernel/src/commands/palette.dart'; import 'package:clide/kernel/src/commands/registry.dart'; +import 'package:clide/kernel/src/reader_nav.dart'; import 'package:clide/kernel/src/dialog.dart'; import 'package:clide/kernel/src/events/bus.dart'; import 'package:clide/kernel/src/events/message_bus.dart'; @@ -63,6 +64,7 @@ abstract class ClideExtensionContext { LayoutArrangement get arrangement; CommandRegistry get commands; PaletteController get palette; + ReaderNavRegistry get readerNav; ClideClipboard get clipboard; FileServices get files; Notifications get notify; diff --git a/lib/kernel/kernel.dart b/lib/kernel/kernel.dart index 4ff2a0ae..e59a8cf3 100644 --- a/lib/kernel/kernel.dart +++ b/lib/kernel/kernel.dart @@ -41,6 +41,7 @@ export 'src/os.dart'; export 'src/panels/arrangement.dart'; export 'src/project.dart'; export 'src/quick_open.dart'; +export 'src/reader_nav.dart'; export 'src/recent_files.dart'; export 'src/scheduler.dart'; export 'src/text_zoom.dart'; diff --git a/lib/kernel/src/extensions_manager.dart b/lib/kernel/src/extensions_manager.dart index e73a0d8a..03da196b 100644 --- a/lib/kernel/src/extensions_manager.dart +++ b/lib/kernel/src/extensions_manager.dart @@ -5,6 +5,7 @@ import 'package:clide/extension/src/extension.dart'; import 'package:clide/kernel/src/clipboard.dart'; import 'package:clide/kernel/src/commands/keybindings.dart'; import 'package:clide/kernel/src/commands/palette.dart'; +import 'package:clide/kernel/src/reader_nav.dart'; import 'package:clide/kernel/src/commands/registry.dart'; import 'package:clide/kernel/src/dialog.dart'; import 'package:clide/kernel/src/events/bus.dart'; @@ -40,6 +41,7 @@ class ExtensionManager extends ChangeNotifier { required this.arrangement, required this.commands, required this.palette, + required this.readerNav, required this.keybindings, required this.keymap, required this.clipboard, @@ -65,6 +67,7 @@ class ExtensionManager extends ChangeNotifier { final LayoutArrangement arrangement; final CommandRegistry commands; final PaletteController palette; + final ReaderNavRegistry readerNav; final KeybindingResolver keybindings; final KeymapService keymap; final ClideClipboard clipboard; @@ -289,6 +292,8 @@ class _ExtensionContext implements ClideExtensionContext { @override PaletteController get palette => manager.palette; @override + ReaderNavRegistry get readerNav => manager.readerNav; + @override ClideClipboard get clipboard => manager.clipboard; @override FileServices get files => manager.files; diff --git a/lib/kernel/src/facade.dart b/lib/kernel/src/facade.dart index a2414dae..4519b069 100644 --- a/lib/kernel/src/facade.dart +++ b/lib/kernel/src/facade.dart @@ -24,6 +24,7 @@ import 'package:clide/kernel/src/panels/arrangement.dart'; import 'package:clide/kernel/src/panels/registry.dart'; import 'package:clide/kernel/src/project.dart'; import 'package:clide/kernel/src/quick_open.dart'; +import 'package:clide/kernel/src/reader_nav.dart'; import 'package:clide/kernel/src/recent_files.dart'; import 'package:clide/kernel/src/scheduler.dart'; import 'package:clide/kernel/src/secrets.dart'; @@ -53,6 +54,7 @@ class KernelServices { required this.palette, required this.quickOpen, required this.recentFiles, + required this.readerNav, required this.keybindings, required this.clipboard, required this.files, @@ -85,6 +87,7 @@ class KernelServices { final PaletteController palette; final QuickOpenController quickOpen; final RecentFilesService recentFiles; + final ReaderNavRegistry readerNav; final KeybindingResolver keybindings; final ClideClipboard clipboard; final FileServices files; @@ -148,6 +151,7 @@ class KernelServices { final palette = PaletteController(commands); final recentFiles = RecentFilesService(); final quickOpen = QuickOpenController(recentPaths: () => recentFiles.paths); + final readerNav = ReaderNavRegistry(messages); final clipboard = ClideClipboard(); final files = FileServices(events); final notify = Notifications(); @@ -196,6 +200,7 @@ class KernelServices { arrangement: arrangement, commands: commands, palette: palette, + readerNav: readerNav, keybindings: keybindings, keymap: keymap, clipboard: clipboard, @@ -229,6 +234,7 @@ class KernelServices { palette: palette, quickOpen: quickOpen, recentFiles: recentFiles, + readerNav: readerNav, keybindings: keybindings, clipboard: clipboard, files: files, @@ -260,6 +266,7 @@ class KernelServices { palette.dispose(); quickOpen.dispose(); recentFiles.dispose(); + readerNav.dispose(); i18n.dispose(); notify.dispose(); dialog.dispose(); diff --git a/lib/kernel/src/reader_nav.dart b/lib/kernel/src/reader_nav.dart new file mode 100644 index 00000000..770339f4 --- /dev/null +++ b/lib/kernel/src/reader_nav.dart @@ -0,0 +1,134 @@ +/// Retained back/forward navigation history for the right-pane readers +/// (markdown, decisions — T-196). +/// +/// The history is the single source of truth for "what the reader is +/// showing", and it OUTLIVES the reader widget — so a reader that mounts +/// after a selection (its tab was just revealed) grabs [ReaderNav.current] +/// instead of missing a broadcast it subscribed to too late. The bus +/// stays a dumb pipe: a [ReaderNav] subscribes to its reader's +/// `selection` channel, records the entry, and (re-)emits `load` — which +/// is the single channel a reader loads from. Back/forward/pin navigation +/// re-emits `load` the same way, so every load flows through one path. +library; + +import 'dart:async'; + +import 'package:clide/kernel/src/events/message_bus.dart'; +import 'package:flutter/foundation.dart'; + +class ReaderNav extends ChangeNotifier { + ReaderNav({ + required MessageBus messages, + required this.publisherId, + required this.dataKey, + }) : _messages = messages { + // Retained recorder: every selection for this reader is captured + // here whether or not the reader widget is mounted. + _selectionSub = _messages.subscribe(publisher: publisherId, channel: 'selection').listen((m) { + final entry = m.data[dataKey]; + if (entry is String) open(entry); + }); + } + + final MessageBus _messages; + + /// The reader's publisher id, e.g. `builtin.decisions` / `builtin.markdown`. + final String publisherId; + + /// The key under which the entry travels in the bus payload (`id` for + /// decisions, `path` for markdown). + final String dataKey; + + StreamSubscription? _selectionSub; + + final List _stack = []; + int _index = -1; + int? _pinnedIndex; + + String? get current => _index >= 0 && _index < _stack.length ? _stack[_index] : null; + bool get canGoBack => _index > 0; + bool get canGoForward => _index < _stack.length - 1; + bool get hasPinned => _pinnedIndex != null && _pinnedIndex! < _stack.length; + + /// External navigation: record [entry] (browser semantics — truncates + /// forward history) and emit a load. A repeat of the current entry + /// re-emits the load without pushing a duplicate. + void open(String entry) { + if (current == entry) { + _emit(entry); + return; + } + if (_index < _stack.length - 1) { + _stack.removeRange(_index + 1, _stack.length); + } + _stack.add(entry); + _index = _stack.length - 1; + _emit(entry); + } + + /// Step back and re-emit the now-current entry. No-op at the start. + void back() { + if (!canGoBack) return; + _index--; + _emit(current!); + } + + /// Step forward and re-emit the now-current entry. No-op at the end. + void forward() { + if (!canGoForward) return; + _index++; + _emit(current!); + } + + /// Pin the current entry (one slot; a later pin replaces it). + void pin() { + if (_index < 0) return; + _pinnedIndex = _index; + notifyListeners(); + } + + /// Jump to the pinned entry and re-emit it. No-op when nothing is pinned. + void jumpToPin() { + final p = _pinnedIndex; + if (p == null || p >= _stack.length) return; + _index = p; + _emit(current!); + } + + void _emit(String entry) { + _messages.publish(publisherId, 'load', {dataKey: entry}); + notifyListeners(); + } + + @override + void dispose() { + _selectionSub?.cancel(); + _selectionSub = null; + super.dispose(); + } +} + +/// Lazily creates and retains one [ReaderNav] per reader id, so the +/// history persists across reader widget mount/unmount. +class ReaderNavRegistry { + ReaderNavRegistry(this._messages); + + final MessageBus _messages; + final Map _navs = {}; + + /// The retained [ReaderNav] for [publisherId], created on first use. + /// [dataKey] is the bus-payload key for this reader's entry. + ReaderNav navFor(String publisherId, {required String dataKey}) { + return _navs.putIfAbsent( + publisherId, + () => ReaderNav(messages: _messages, publisherId: publisherId, dataKey: dataKey), + ); + } + + void dispose() { + for (final n in _navs.values) { + n.dispose(); + } + _navs.clear(); + } +} diff --git a/test/builtin/decisions/decision_reader_test.dart b/test/builtin/decisions/decision_reader_test.dart index aeffe680..d672b9d7 100644 --- a/test/builtin/decisions/decision_reader_test.dart +++ b/test/builtin/decisions/decision_reader_test.dart @@ -89,7 +89,13 @@ void main() { f = await KernelFixture.create(); await _bootExtension(f); }); - tearDown(() => f.dispose()); + tearDown(() async { + // Deactivate before dispose so any post-frame forward scheduled by + // these (non-pumping) tests is neutralised — otherwise it fires in + // a later testWidgets against a torn-down bus. + await f.services.extensions.deactivate('builtin.decisions'); + await f.dispose(); + }); test('activate contributes decisions.detail as a static tab', () { final tabs = f.services.panels.tabsFor(Slots.contextPanel); @@ -237,8 +243,9 @@ void main() { // Starts empty. expect(find.text('Select a decision to view details.'), findsOneWidget); - // Publish a selection. - f.services.messages.publish('builtin.decisions', 'selection', {'id': 'D-7'}); + // The view loads on 'load' (forwarded by the extension post-frame + // after it reveals the tab; T-196). + f.services.messages.publish('builtin.decisions', 'load', {'id': 'D-7'}); // Give the broadcast stream a microtask to deliver. await pumpAsync(tester); @@ -251,7 +258,7 @@ void main() { await pumpView(tester, initialId: 'D-1'); expect(find.text('Decision D-1'), findsWidgets); - f.services.messages.publish('builtin.decisions', 'selection', {'id': 'D-2'}); + f.services.messages.publish('builtin.decisions', 'load', {'id': 'D-2'}); await pumpAsync(tester); expect(find.text('Decision D-2'), findsWidgets); @@ -262,7 +269,7 @@ void main() { await pumpView(tester, initialId: 'D-5'); expect(find.text('Decision D-5'), findsWidgets); - f.services.messages.publish('builtin.decisions', 'selection', {'id': 'D-5'}); + f.services.messages.publish('builtin.decisions', 'load', {'id': 'D-5'}); await pumpAsync(tester); // Still shows D-5, no crash. @@ -273,7 +280,7 @@ void main() { await pumpView(tester); for (var i = 1; i <= 5; i++) { - f.services.messages.publish('builtin.decisions', 'selection', {'id': 'D-$i'}); + f.services.messages.publish('builtin.decisions', 'load', {'id': 'D-$i'}); } await pumpAsync(tester); @@ -541,6 +548,13 @@ void main() { }); tearDown(() => f.dispose()); + // Drive the retained nav (the history source); its 'load' emit makes + // the mounted view display the entry (T-196). + Future open(WidgetTester tester, String id) async { + f.services.readerNav.navFor('builtin.decisions', dataKey: 'id').open(id); + await pumpAsync(tester); + } + Future pumpView(WidgetTester tester, {String? initialId}) async { tester.view.physicalSize = const Size(600, 800); tester.view.devicePixelRatio = 1.0; @@ -548,8 +562,9 @@ void main() { tester.view.resetPhysicalSize(); tester.view.resetDevicePixelRatio(); }); - await tester.pumpWidget(harness(f, DecisionDetailView(initialId: initialId))); + await tester.pumpWidget(harness(f, const DecisionDetailView())); await pumpAsync(tester); + if (initialId != null) await open(tester, initialId); } testWidgets('back disabled on initial load', (tester) async { @@ -565,8 +580,7 @@ void main() { testWidgets('back enabled after two selections', (tester) async { await pumpView(tester, initialId: 'D-1'); - f.services.messages.publish('builtin.decisions', 'selection', {'id': 'D-2'}); - await pumpAsync(tester); + await open(tester, 'D-2'); expect( find.byWidgetPredicate( @@ -578,8 +592,7 @@ void main() { testWidgets('back navigates to previous decision', (tester) async { await pumpView(tester, initialId: 'D-1'); - f.services.messages.publish('builtin.decisions', 'selection', {'id': 'D-2'}); - await pumpAsync(tester); + await open(tester, 'D-2'); // Title appears in pane header subtitle + body card. expect(find.text('Decision D-2'), findsWidgets); @@ -594,14 +607,13 @@ void main() { testWidgets('back/forward does NOT re-publish selection bus event', (tester) async { await pumpView(tester, initialId: 'D-1'); - f.services.messages.publish('builtin.decisions', 'selection', {'id': 'D-2'}); - await pumpAsync(tester); + await open(tester, 'D-2'); final selections = []; final sub = f.services.messages.subscribe(publisher: 'builtin.decisions', channel: 'selection').listen(selections.add); addTearDown(sub.cancel); - // Go back — should NOT publish a selection message. + // Go back — re-emits on 'load', NOT 'selection'. final backBtn = find.byWidgetPredicate( (w) => w is Semantics && w.properties.label == 'Back' && (w.properties.enabled ?? true), ); @@ -613,8 +625,7 @@ void main() { testWidgets('forward disabled at end of history', (tester) async { await pumpView(tester, initialId: 'D-1'); - f.services.messages.publish('builtin.decisions', 'selection', {'id': 'D-2'}); - await pumpAsync(tester); + await open(tester, 'D-2'); expect( find.byWidgetPredicate( @@ -626,8 +637,7 @@ void main() { testWidgets('forward navigates after back', (tester) async { await pumpView(tester, initialId: 'D-1'); - f.services.messages.publish('builtin.decisions', 'selection', {'id': 'D-2'}); - await pumpAsync(tester); + await open(tester, 'D-2'); // Go back to D-1. final backBtn = find.byWidgetPredicate( @@ -648,8 +658,7 @@ void main() { testWidgets('new selection truncates forward history', (tester) async { await pumpView(tester, initialId: 'D-1'); - f.services.messages.publish('builtin.decisions', 'selection', {'id': 'D-2'}); - await pumpAsync(tester); + await open(tester, 'D-2'); // Go back to D-1. final backBtn = find.byWidgetPredicate( @@ -658,9 +667,8 @@ void main() { await tester.tap(backBtn.first); await pumpAsync(tester); - // Load D-3 — truncates D-2 forward history. - f.services.messages.publish('builtin.decisions', 'selection', {'id': 'D-3'}); - await pumpAsync(tester); + // Open D-3 — truncates D-2 forward history. + await open(tester, 'D-3'); expect( find.byWidgetPredicate( @@ -687,6 +695,11 @@ void main() { }); tearDown(() => f.dispose()); + Future open(WidgetTester tester, String id) async { + f.services.readerNav.navFor('builtin.decisions', dataKey: 'id').open(id); + await pumpAsync(tester); + } + Future pumpView(WidgetTester tester, {String? initialId}) async { tester.view.physicalSize = const Size(600, 800); tester.view.devicePixelRatio = 1.0; @@ -694,8 +707,9 @@ void main() { tester.view.resetPhysicalSize(); tester.view.resetDevicePixelRatio(); }); - await tester.pumpWidget(harness(f, DecisionDetailView(initialId: initialId))); + await tester.pumpWidget(harness(f, const DecisionDetailView())); await pumpAsync(tester); + if (initialId != null) await open(tester, initialId); } testWidgets('pin jump affordance not visible before pin set', (tester) async { @@ -734,8 +748,7 @@ void main() { await pumpAsync(tester); // Navigate to D-2. - f.services.messages.publish('builtin.decisions', 'selection', {'id': 'D-2'}); - await pumpAsync(tester); + await open(tester, 'D-2'); // Title appears in pane header subtitle + body card. expect(find.text('Decision D-2'), findsWidgets); @@ -762,8 +775,7 @@ void main() { await pumpAsync(tester); // Navigate to D-2. - f.services.messages.publish('builtin.decisions', 'selection', {'id': 'D-2'}); - await pumpAsync(tester); + await open(tester, 'D-2'); // Replace pin with D-2. await tester.tap(find @@ -774,8 +786,7 @@ void main() { await pumpAsync(tester); // Navigate to D-3. - f.services.messages.publish('builtin.decisions', 'selection', {'id': 'D-3'}); - await pumpAsync(tester); + await open(tester, 'D-3'); // Jump to pin — should go to D-2 (replaced), not D-1. await tester.tap(find diff --git a/test/builtin/editor/editor_extension_test.dart b/test/builtin/editor/editor_extension_test.dart new file mode 100644 index 00000000..459a8fc2 --- /dev/null +++ b/test/builtin/editor/editor_extension_test.dart @@ -0,0 +1,75 @@ +/// T-197: EditorExtension reveals its workspace tab when a buffer opens. +/// +/// `editor.open` opens the buffer daemon-side and emits `editor.opened`, +/// but nothing else brings the editor tab to front over the Claude +/// pane. The extension's activate() listens for the editor lifecycle +/// events and activates the workspace tab. +library; + +import 'package:clide/builtin/editor/src/extension.dart'; +import 'package:clide/extension/extension.dart' show TabContribution; +import 'package:clide/kernel/kernel.dart'; +import 'package:flutter/widgets.dart'; +import 'package:flutter_test/flutter_test.dart'; + +import '../../helpers/kernel_fixture.dart'; + +void main() { + late KernelFixture f; + + setUp(() async { + f = await KernelFixture.create(); + f.services.panels.registerSlot(const SlotDefinition(id: Slots.workspace, position: SlotPosition.center)); + // A pre-existing workspace tab so 'editor.active' is NOT the default + // active tab — the reveal must switch to it explicitly. + f.services.panels.contribute(TabContribution( + id: 'claude.primary', + slot: Slots.workspace, + title: 'Claude', + build: (_) => const SizedBox(), + )); + f.services.extensions.register(EditorExtension()); + await f.services.extensions.activate('builtin.editor'); + }); + tearDown(() => f.dispose()); + + void emitEditor(String kind, {String? id}) { + f.services.events.emit(DaemonEvent(subsystem: 'editor', kind: kind, data: {'id': id}, ts: DateTime.now().toUtc())); + } + + test('contributes editor.active but leaves Claude active by default', () { + expect(f.services.panels.tabsFor(Slots.workspace).any((t) => t.id == 'editor.active'), isTrue); + expect(f.services.panels.activeTabIn(Slots.workspace), 'claude.primary'); + }); + + test('editor.opened reveals (activates) the editor tab', () async { + emitEditor('editor.opened', id: 'b_1'); + await Future.delayed(Duration.zero); + expect(f.services.panels.activeTabIn(Slots.workspace), 'editor.active'); + }); + + test('editor.active-changed also reveals the editor tab', () async { + emitEditor('editor.active-changed', id: 'b_2'); + await Future.delayed(Duration.zero); + expect(f.services.panels.activeTabIn(Slots.workspace), 'editor.active'); + }); + + test('a non-editor event leaves the active tab unchanged', () async { + f.services.events.emit(DaemonEvent(subsystem: 'git', kind: 'changed', data: const {}, ts: DateTime.now().toUtc())); + await Future.delayed(Duration.zero); + expect(f.services.panels.activeTabIn(Slots.workspace), 'claude.primary'); + }); + + test('an unrelated editor event kind does not reveal', () async { + emitEditor('editor.saved', id: 'b_1'); + await Future.delayed(Duration.zero); + expect(f.services.panels.activeTabIn(Slots.workspace), 'claude.primary'); + }); + + test('after deactivate, editor events no longer reveal', () async { + await f.services.extensions.deactivate('builtin.editor'); + emitEditor('editor.opened', id: 'b_9'); + await Future.delayed(Duration.zero); + expect(f.services.panels.activeTabIn(Slots.workspace), isNot('editor.active')); + }); +} diff --git a/test/builtin/markdown/markdown_viewer_test.dart b/test/builtin/markdown/markdown_viewer_test.dart index 41c098c7..fae9db1e 100644 --- a/test/builtin/markdown/markdown_viewer_test.dart +++ b/test/builtin/markdown/markdown_viewer_test.dart @@ -50,9 +50,10 @@ Future pumpView(WidgetTester tester, KernelFixture f) async { await pumpAsync(tester); } -/// Trigger a file load via the 'load' channel (mirrors the extension bridge). +/// Open a file through the retained nav (the history source); its 'load' +/// emit makes the mounted viewer display it (T-196). Future loadFile(WidgetTester tester, KernelFixture f, String path) async { - f.services.messages.publish('builtin.markdown', 'load', {'path': path}); + f.services.readerNav.navFor('builtin.markdown', dataKey: 'path').open(path); await pumpAsync(tester); } diff --git a/test/builtin/shared/reader_chrome_test.dart b/test/builtin/shared/reader_chrome_test.dart deleted file mode 100644 index 66d9e8fe..00000000 --- a/test/builtin/shared/reader_chrome_test.dart +++ /dev/null @@ -1,222 +0,0 @@ -/// Unit tests for [ReaderHistory] and [ReaderHistoryMixin] (T-189, T-190). -library; - -import 'package:clide/builtin/shared/reader_chrome.dart'; -import 'package:flutter/widgets.dart'; -import 'package:flutter_test/flutter_test.dart'; - -// --------------------------------------------------------------------------- -// ReaderHistory unit tests (no Flutter needed — plain test()) -// --------------------------------------------------------------------------- - -void main() { - group('ReaderHistory', () { - late ReaderHistory h; - - setUp(() => h = ReaderHistory()); - - test('starts empty — canGoBack/Forward false, current null', () { - expect(h.canGoBack, isFalse); - expect(h.canGoForward, isFalse); - expect(h.current, isNull); - }); - - test('push one — current is that entry, no back/forward', () { - h.push('A'); - expect(h.current, 'A'); - expect(h.canGoBack, isFalse); - expect(h.canGoForward, isFalse); - }); - - test('push two — canGoBack true, canGoForward false', () { - h.push('A'); - h.push('B'); - expect(h.current, 'B'); - expect(h.canGoBack, isTrue); - expect(h.canGoForward, isFalse); - }); - - test('back() after two pushes returns first entry', () { - h.push('A'); - h.push('B'); - final result = h.back(); - expect(result, 'A'); - expect(h.current, 'A'); - expect(h.canGoBack, isFalse); - expect(h.canGoForward, isTrue); - }); - - test('forward() after back() returns second entry', () { - h.push('A'); - h.push('B'); - h.back(); - final result = h.forward(); - expect(result, 'B'); - expect(h.current, 'B'); - expect(h.canGoForward, isFalse); - }); - - test('back() at start returns null', () { - h.push('A'); - expect(h.back(), isNull); - }); - - test('forward() at end returns null', () { - h.push('A'); - h.push('B'); - expect(h.forward(), isNull); - }); - - test('new push truncates forward history', () { - h.push('A'); - h.push('B'); - h.push('C'); - h.back(); // now at B - h.back(); // now at A - expect(h.canGoForward, isTrue); - - h.push('D'); // truncates [B, C], appends D - expect(h.current, 'D'); - expect(h.canGoBack, isTrue); - expect(h.canGoForward, isFalse); - - final prev = h.back(); - expect(prev, 'A'); - }); - - test('pushing duplicate of current is a no-op', () { - h.push('A'); - h.push('A'); - expect(h.canGoBack, isFalse); // still only one entry - expect(h.current, 'A'); - }); - - test('three entries back/forward round-trip', () { - h.push('A'); - h.push('B'); - h.push('C'); - expect(h.back(), 'B'); - expect(h.back(), 'A'); - expect(h.forward(), 'B'); - expect(h.forward(), 'C'); - expect(h.canGoForward, isFalse); - }); - }); - - // ------------------------------------------------------------------------- - // ReaderHistoryMixin widget integration test — uses a minimal StatefulWidget. - // ------------------------------------------------------------------------- - - group('ReaderHistoryMixin', () { - testWidgets('pin current / jump-to-pin round-trip', (tester) async { - String? jumpedTo; - - await tester.pumpWidget( - Directionality( - textDirection: TextDirection.ltr, - child: _MixinHarness(onJump: (v) => jumpedTo = v), - ), - ); - - final state = tester.state<_MixinHarnessState>(find.byType(_MixinHarness)); - - // No pin yet. - expect(state.hasPinned, isFalse); - expect(state.pinnedEntry, isNull); - - // Push 'A', then pin it. - state.historyPush('A'); - await tester.pump(); - state.pinCurrent(); - await tester.pump(); - - expect(state.hasPinned, isTrue); - expect(state.pinnedEntry, 'A'); - - // Push 'B', jump to pin → should get 'A'. - state.historyPush('B'); - await tester.pump(); - final pinEntry = state.jumpToPin(); - jumpedTo = pinEntry; - expect(jumpedTo, 'A'); - }); - - testWidgets('pin replaces previous pin', (tester) async { - await tester.pumpWidget( - Directionality( - textDirection: TextDirection.ltr, - child: _MixinHarness(onJump: (_) {}), - ), - ); - - final state = tester.state<_MixinHarnessState>(find.byType(_MixinHarness)); - state.historyPush('A'); - state.pinCurrent(); - await tester.pump(); - expect(state.pinnedEntry, 'A'); - - state.historyPush('B'); - state.pinCurrent(); - await tester.pump(); - expect(state.pinnedEntry, 'B'); // replaced - }); - - testWidgets('historyBack / historyForward returns correct entries', (tester) async { - await tester.pumpWidget( - Directionality( - textDirection: TextDirection.ltr, - child: _MixinHarness(onJump: (_) {}), - ), - ); - - final state = tester.state<_MixinHarnessState>(find.byType(_MixinHarness)); - state.historyPush('X'); - state.historyPush('Y'); - await tester.pump(); - - expect(state.canGoBack, isTrue); - expect(state.canGoForward, isFalse); - - final back = state.historyBack(); - await tester.pump(); - expect(back, 'X'); - expect(state.canGoBack, isFalse); - expect(state.canGoForward, isTrue); - - final fwd = state.historyForward(); - await tester.pump(); - expect(fwd, 'Y'); - }); - - testWidgets('pinCurrent with empty history is a no-op', (tester) async { - await tester.pumpWidget( - Directionality( - textDirection: TextDirection.ltr, - child: _MixinHarness(onJump: (_) {}), - ), - ); - - final state = tester.state<_MixinHarnessState>(find.byType(_MixinHarness)); - state.pinCurrent(); // no current entry — must not throw - await tester.pump(); - expect(state.hasPinned, isFalse); - }); - }); -} - -// --------------------------------------------------------------------------- -// Minimal harness widget that mixes in ReaderHistoryMixin. -// --------------------------------------------------------------------------- - -class _MixinHarness extends StatefulWidget { - const _MixinHarness({required this.onJump}); - final void Function(String?) onJump; - - @override - State<_MixinHarness> createState() => _MixinHarnessState(); -} - -class _MixinHarnessState extends State<_MixinHarness> with ReaderHistoryMixin { - @override - Widget build(BuildContext context) => const SizedBox.shrink(); -} diff --git a/test/kernel/reader_nav_test.dart b/test/kernel/reader_nav_test.dart new file mode 100644 index 00000000..ad414e74 --- /dev/null +++ b/test/kernel/reader_nav_test.dart @@ -0,0 +1,137 @@ +/// Unit tests for the retained right-pane nav history (T-196). +/// +/// [ReaderNav] records selections (even before the reader mounts), holds +/// browser-style back/forward history, exposes the latest as [current] +/// for grab-on-mount, and re-emits every navigation on the `load` +/// channel so the reader has a single load path. +library; + +import 'package:clide/kernel/src/events/message_bus.dart'; +import 'package:clide/kernel/src/reader_nav.dart'; +import 'package:flutter_test/flutter_test.dart'; + +void main() { + late MessageBus bus; + late ReaderNav nav; + late List loads; + + setUp(() { + bus = MessageBus(); + nav = ReaderNav(messages: bus, publisherId: 'builtin.decisions', dataKey: 'id'); + loads = []; + bus.subscribe(publisher: 'builtin.decisions', channel: 'load').listen((m) { + final id = m.data['id']; + if (id is String) loads.add(id); + }); + }); + tearDown(() { + nav.dispose(); + bus.dispose(); + }); + + // Let the broadcast bus deliver. + Future tick() => Future.delayed(Duration.zero); + + test('starts empty', () { + expect(nav.current, isNull); + expect(nav.canGoBack, isFalse); + expect(nav.canGoForward, isFalse); + expect(nav.hasPinned, isFalse); + }); + + test('open records the entry, sets current, and emits a load', () async { + nav.open('D-1'); + expect(nav.current, 'D-1'); + expect(nav.canGoBack, isFalse); + await tick(); + expect(loads, ['D-1']); + }); + + test('a selection on the bus is recorded (retained) and emits a load', () async { + bus.publish('builtin.decisions', 'selection', {'id': 'D-9'}); + await tick(); + expect(nav.current, 'D-9'); + expect(loads, ['D-9']); + }); + + test('two opens enable back; back re-emits the prior entry', () async { + nav.open('D-1'); + nav.open('D-2'); + expect(nav.canGoBack, isTrue); + expect(nav.canGoForward, isFalse); + nav.back(); + expect(nav.current, 'D-1'); + expect(nav.canGoForward, isTrue); + await tick(); + expect(loads, ['D-1', 'D-2', 'D-1']); + }); + + test('forward after back re-emits the later entry', () async { + nav.open('D-1'); + nav.open('D-2'); + nav.back(); + nav.forward(); + expect(nav.current, 'D-2'); + }); + + test('back at start / forward at end are no-ops', () async { + nav.open('D-1'); + nav.back(); // canGoBack false → no-op + nav.forward(); // canGoForward false → no-op + await tick(); + expect(loads, ['D-1']); // only the open emitted + expect(nav.current, 'D-1'); + }); + + test('a new open truncates forward history', () { + nav.open('D-1'); + nav.open('D-2'); + nav.open('D-3'); + nav.back(); // D-2 + nav.back(); // D-1 + nav.open('D-9'); // truncates D-2/D-3 + expect(nav.current, 'D-9'); + expect(nav.canGoForward, isFalse); + expect(nav.canGoBack, isTrue); + }); + + test('opening the current entry again re-emits but does not push', () async { + nav.open('D-1'); + nav.open('D-1'); + expect(nav.canGoBack, isFalse); // no duplicate pushed + await tick(); + expect(loads, ['D-1', 'D-1']); // but both re-emit a load + }); + + test('pin + jumpToPin returns to the pinned entry and re-emits', () async { + nav.open('D-1'); + nav.pin(); + expect(nav.hasPinned, isTrue); + nav.open('D-2'); + nav.open('D-3'); + nav.jumpToPin(); + expect(nav.current, 'D-1'); + await tick(); + expect(loads.last, 'D-1'); + }); + + test('pin replaces the previous pin', () { + nav.open('D-1'); + nav.pin(); + nav.open('D-2'); + nav.pin(); // replaces + nav.open('D-3'); + nav.jumpToPin(); + expect(nav.current, 'D-2'); + }); + + test('registry retains one nav per reader id', () { + final reg = ReaderNavRegistry(bus); + addTearDown(reg.dispose); + final a = reg.navFor('builtin.markdown', dataKey: 'path'); + final b = reg.navFor('builtin.markdown', dataKey: 'path'); + expect(identical(a, b), isTrue); + final c = reg.navFor('builtin.decisions', dataKey: 'id'); + expect(identical(a, c), isFalse); + }); +}