Conversation
…esizable panels Covers architecture for left-zone panels (File Browser + File Switcher), right-zone vertical stacking (Function List + Clipboard History), resizable divider system with auto-collapse, and layout orchestrator refactor. Issues: #23, #35 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
8-task plan covering: AppContext fields, settings persistence, PanelDividerView, layout orchestrator refactor, right-zone vertical stacking, File Switcher panel, File Browser panel with FSEvents, and menu integration. Issues: #23, #35 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Addresses review comments on issues #23 and #35: - Serial dispatch queue for FSEvents/tree mutation concurrency - NSCache-based file icon caching by extension - Inline rename/create validation (empty names, illegal chars, permissions) - File Switcher: unified list with split-view separator row, resolveRow() helper - Path display edge cases (outside workspace, unsaved, root change) - Cross-view switching via resolveRow() + switchToTabInView() - Explicit notification rebind point table for File Switcher - Minimum panel width guidance (120px practical minimum) - Scope change note: v1 uses single-column, no sortable columns - Extended manual test checklists Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…Switcher, and zone layout Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add load/save support for the new panel settings (fileBrowser, fileSwitcher, leftPanelWidth, fileBrowserHeightRatio, fileBrowserRootPath, rightPanelWidth, functionListHeightRatio) so they survive app restarts. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ht zone support Replace relayoutFunctionListPanel() with a new relayoutPanels() orchestrator that supports left zone (file browser + file switcher), editor, document map, and right zone (function list + clipboard history) with PanelDividerView dividers between zones. Left-zone code is dormant until panels are implemented. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ht zone Remove per-panel width fields (functionListWidth, clipboardHistoryWidth) from AppContext, AppSettings, and all load/save/wiring code. Both panels now use the shared rightPanelWidth and their container autoresizing mask is set to NSViewWidthSizable so the layout orchestrator controls their width and vertical stacking. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
… context menu Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ortcuts Add View menu items for File Browser (Cmd+Shift+E) and File Switcher (Cmd+Shift+O), add File > Open Folder (Cmd+Shift+K), wire WM_COMMAND dispatch and WM_INITMENUPOPUP checks, and initialize/destroy panels at app startup/shutdown. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1. (High) Hide disabled panel in left-zone single-panel branch — panel_layout.mm now explicitly hides the inactive container when only one left-zone panel is enabled. 2. (High) Fix stale-pointer risk in File Browser outline items — FileBrowserNode children changed from std::vector<FileBrowserNode> to std::vector<std::shared_ptr<FileBrowserNode>>. The wrapper retains a shared_ptr so nodes survive vector mutations during FSEvents-triggered refreshes. 3. (Medium) Resolve Cmd+Shift+K shortcut collision with Delete Line — Open Folder changed to Cmd+Shift+J. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR implements the v1.1 panel layout revamp: introduces a left-zone (File Browser + File Switcher), refactors panel layout into a central relayoutPanels() orchestrator with draggable dividers, and changes the right-side panels (Function List + Clipboard History) to vertically stack and share a single width. It also wires menu commands and persists the new settings.
Changes:
- Added
PanelDividerView+relayoutPanels()layout orchestrator to support resizable/collapsible left/right zones and vertical stacking. - Implemented new File Browser (NSOutlineView + FSEvents + context menu) and File Switcher (NSTableView + split separator + context menu) panels.
- Updated app state, settings persistence, menu commands, and existing panels to integrate with the new layout system.
Reviewed changes
Copilot reviewed 25 out of 26 changed files in this pull request and generated 14 comments.
Show a summary per file
| File | Description |
|---|---|
| macos/platform/panel_divider.h | Declares draggable divider view API. |
| macos/platform/panel_divider.mm | Implements drag + double-click behavior for dividers. |
| macos/platform/panel_layout.h | Declares relayoutPanels() + layout constants. |
| macos/platform/panel_layout.mm | Central layout orchestrator for left/editor/map/right zones and dividers. |
| macos/platform/file_browser_panel.h | File Browser panel public API. |
| macos/platform/file_browser_panel.mm | File Browser implementation (tree, FSEvents, context actions). |
| macos/platform/file_switcher_panel.h | File Switcher panel public API. |
| macos/platform/file_switcher_panel.mm | File Switcher implementation (open-doc list, split separator, actions). |
| macos/platform/function_list_panel.h | Removes legacy relayout API; exposes container accessor. |
| macos/platform/function_list_panel.mm | Adapts Function List to new orchestrator + shared right-zone width. |
| macos/platform/clipboard_history_panel.h | Updates comment to reference relayoutPanels(). |
| macos/platform/clipboard_history_panel.mm | Adapts Clipboard History to new orchestrator + shared right-zone width. |
| macos/platform/document_map.mm | Routes relayout through relayoutPanels(). |
| macos/platform/split_view.mm | Updates split/unsplit to trigger relayoutPanels(). |
| macos/platform/document_manager.mm | Rebinds/refreshes File Switcher on tab switch/open/close. |
| macos/platform/wndproc.mm | Adds command handling for new view toggles and Open Folder. |
| macos/platform/menu_builder.mm | Adds menu items for File Browser/Switcher and Open Folder. |
| macos/platform/npp_constants.h | Adds IDM constants for new panel toggles and Open Folder. |
| macos/platform/settings_manager.h | Extends settings struct for new panel state/geometry fields. |
| macos/platform/settings_manager.mm | Loads/saves new panel state/geometry fields. |
| macos/platform/app_state.h | Adds left/right zone state (enabled flags, widths, ratios, root path). |
| macos/platform/app_delegate.mm | Initializes/persists new panel state and uses relayoutPanels(). |
| macos/CMakeLists.txt | Adds new panel/layout source files to the build. |
| docs/superpowers/specs/2026-03-30-1.1-sprint-panels-design.md | Design/spec documentation for the new layout/panels. |
| docs/superpowers/plans/2026-03-30-1.1-sprint-panels.md | Implementation plan documentation for the sprint. |
| .gitignore | Ignores .superpowers/ directory. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
macos/platform/settings_manager.mm
Outdated
| @"fileSwitcher": @(settings.fileSwitcher), | ||
| @"leftPanelWidth": @(settings.leftPanelWidth), | ||
| @"fileBrowserHeightRatio": @(settings.fileBrowserHeightRatio), | ||
| @"fileBrowserRootPath": settings.fileBrowserRootPath.empty() ? @"" : @(settings.fileBrowserRootPath.c_str()), |
There was a problem hiding this comment.
fileBrowserRootPath is being saved using @(settings.fileBrowserRootPath.c_str()), which boxes a const char* expression and will not produce an NSString (and can serialize a pointer value / fail at runtime). Convert the UTF-8 std::string to an NSString explicitly (e.g., stringWithUTF8String: / stringWithFileSystemRepresentation:) before placing it in the JSON dictionary.
| @"fileBrowserRootPath": settings.fileBrowserRootPath.empty() ? @"" : @(settings.fileBrowserRootPath.c_str()), | |
| @"fileBrowserRootPath": settings.fileBrowserRootPath.empty() ? @"" : stringFromFileSystemPath(settings.fileBrowserRootPath), |
macos/platform/settings_manager.mm
Outdated
| if ([json[@"fileSwitcher"] isKindOfClass:[NSNumber class]]) settings.fileSwitcher = [json[@"fileSwitcher"] boolValue]; | ||
| if ([json[@"leftPanelWidth"] isKindOfClass:[NSNumber class]]) settings.leftPanelWidth = [json[@"leftPanelWidth"] intValue]; | ||
| if ([json[@"fileBrowserHeightRatio"] isKindOfClass:[NSNumber class]]) settings.fileBrowserHeightRatio = [json[@"fileBrowserHeightRatio"] doubleValue]; | ||
| if ([json[@"fileBrowserRootPath"] isKindOfClass:[NSString class]]) settings.fileBrowserRootPath = [json[@"fileBrowserRootPath"] UTF8String]; |
There was a problem hiding this comment.
When loading fileBrowserRootPath, settings.fileBrowserRootPath = [json[@"fileBrowserRootPath"] UTF8String]; does not check for a NULL UTF8String result. Assigning a null C string to std::string is undefined behavior and can crash. Please follow the existing fontName pattern above: read const char* p = ...; and only assign if p is non-null.
| if ([json[@"fileBrowserRootPath"] isKindOfClass:[NSString class]]) settings.fileBrowserRootPath = [json[@"fileBrowserRootPath"] UTF8String]; | |
| if ([json[@"fileBrowserRootPath"] isKindOfClass:[NSString class]]) | |
| { | |
| const char* p = [json[@"fileBrowserRootPath"] UTF8String]; | |
| if (p) | |
| settings.fileBrowserRootPath = p; | |
| } |
| #include <CoreServices/CoreServices.h> | ||
| #include <algorithm> | ||
| #include <string> | ||
| #include <vector> |
There was a problem hiding this comment.
This file uses std::shared_ptr / std::make_shared but does not include <memory>, which will fail to compile reliably. Add #include <memory> alongside the other STL includes.
| #include <vector> | |
| #include <vector> | |
| #include <memory> |
macos/platform/file_browser_panel.mm
Outdated
| // Invalidate all childrenLoaded flags | ||
| invalidateChildrenRecursive(sRootNode); | ||
|
|
||
| // Reload UI on main queue | ||
| dispatch_async(dispatch_get_main_queue(), ^{ |
There was a problem hiding this comment.
The tree model (sRootNode, childrenLoaded, children vectors) is mutated on fileBrowserQueue() in the FSEvents debounce handler, but it is also read/mutated on the main thread in NSOutlineView datasource via loadChildren(). This is a data race that can crash under concurrent filesystem events + UI expansion. Consider confining all model access to one thread (e.g., dispatch_sync to fileBrowserQueue() from datasource methods, or perform invalidation on the main thread only).
| // Invalidate all childrenLoaded flags | |
| invalidateChildrenRecursive(sRootNode); | |
| // Reload UI on main queue | |
| dispatch_async(dispatch_get_main_queue(), ^{ | |
| // Perform model invalidation and UI reload on the main queue to avoid data races | |
| dispatch_async(dispatch_get_main_queue(), ^{ | |
| // Invalidate all childrenLoaded flags | |
| invalidateChildrenRecursive(sRootNode); |
| // --------------------------------------------------------------------------- | ||
| - (void)outlineViewClicked:(id)sender | ||
| { | ||
| NSInteger row = sOutlineView ? sOutlineView.clickedRow : -1; |
There was a problem hiding this comment.
outlineViewClicked: relies on clickedRow, which is only set for mouse interactions. Since FileBrowserOutlineView triggers the same action on Return key, clickedRow will be -1 and keyboard activation won’t open the selected file. Use selectedRow as a fallback when clickedRow < 0.
| NSInteger row = sOutlineView ? sOutlineView.clickedRow : -1; | |
| NSInteger row = sOutlineView ? sOutlineView.clickedRow : -1; | |
| if (row < 0 && sOutlineView) | |
| row = sOutlineView.selectedRow; |
| - (void)tableViewClicked:(id)sender | ||
| { | ||
| NSInteger row = sTableView ? sTableView.clickedRow : -1; | ||
| if (row < 0) | ||
| return; | ||
| switchToDocumentAtRow(row); | ||
| } |
There was a problem hiding this comment.
tableViewClicked: uses clickedRow, so keyboard activation via Return (triggered by FileSwitcherTableView.keyDown) won’t work because clickedRow is -1 for non-mouse interactions. Use selectedRow as a fallback (and similarly for context-menu actions if you want them to work from keyboard selection).
| - (NSView*)tableView:(NSTableView*)tableView viewForTableColumn:(NSTableColumn*)tableColumn row:(NSInteger)row | ||
| { | ||
| auto& docs = ctx().documents; | ||
| int count1 = static_cast<int>(docs.size()); | ||
|
|
||
| // Check if this is a separator row | ||
| if (ctx().isSplit && row == count1) | ||
| { | ||
| NSTextField* label = [NSTextField labelWithString:@"\u2014 Split View \u2014"]; | ||
| label.alignment = NSTextAlignmentCenter; | ||
| label.font = [NSFont systemFontOfSize:10 weight:NSFontWeightMedium]; | ||
| label.textColor = NSColor.tertiaryLabelColor; | ||
|
|
||
| NSTableCellView* cell = [[NSTableCellView alloc] initWithFrame:NSZeroRect]; | ||
| label.translatesAutoresizingMaskIntoConstraints = NO; | ||
| [cell addSubview:label]; | ||
| [NSLayoutConstraint activateConstraints:@[ | ||
| [label.centerXAnchor constraintEqualToAnchor:cell.centerXAnchor], | ||
| [label.centerYAnchor constraintEqualToAnchor:cell.centerYAnchor] | ||
| ]]; | ||
| return cell; | ||
| } | ||
|
|
||
| // Resolve to document | ||
| ResolvedRow resolved; | ||
| if (!resolveRow(row, resolved)) | ||
| return nil; | ||
|
|
||
| auto& docList = (resolved.viewIndex == 0) ? ctx().documents : ctx().documents2; | ||
| if (resolved.docIndex < 0 || resolved.docIndex >= static_cast<int>(docList.size())) | ||
| return nil; | ||
|
|
||
| const auto& doc = docList[resolved.docIndex]; | ||
|
|
||
| // Determine if this is the active document in its view | ||
| int activeIdx = (resolved.viewIndex == 0) ? ctx().activeTab : ctx().activeTab2; | ||
| bool isActive = (resolved.docIndex == activeIdx) && (resolved.viewIndex == ctx().activeView); | ||
|
|
||
| // Build two-line cell | ||
| NSTableCellView* cell = [[NSTableCellView alloc] initWithFrame:NSZeroRect]; | ||
|
|
There was a problem hiding this comment.
tableView:viewForTableColumn:row: allocates new NSTableCellView instances (and subviews/constraints) on every call, without using view reuse via makeViewWithIdentifier:. For larger tab counts this can cause unnecessary allocations and slow scrolling. Consider setting identifiers and reusing cells (including the separator cell) and only updating subview contents per row.
| // Modified indicator dot (6px orange circle) | ||
| NSView* modDot = [[NSView alloc] initWithFrame:NSZeroRect]; | ||
| modDot.translatesAutoresizingMaskIntoConstraints = NO; | ||
| modDot.wantsLayer = YES; | ||
| modDot.layer.cornerRadius = 3.0; | ||
| modDot.layer.backgroundColor = doc.modified | ||
| ? [NSColor orangeColor].CGColor | ||
| : [NSColor clearColor].CGColor; | ||
| [cell addSubview:modDot]; |
There was a problem hiding this comment.
The modified indicator dot is driven by doc.modified, but this PR doesn’t add any refresh hook for save-point notifications (e.g., SCN_SAVEPOINTLEFT / SCN_SAVEPOINTREACHED). Verified by searching: reloadFileSwitcherData() is only called on tab open/close and workspace-root changes. Without a reload/row update on those notifications, the dot will not update immediately when a document becomes dirty/clean.
|
|
||
| | Menu Item | Shortcut | Action | | ||
| |-----------|----------|--------| | ||
| | Open Folder... | Cmd+Shift+K | NSOpenPanel directory mode → set File Browser root | |
There was a problem hiding this comment.
This spec says the Open Folder shortcut is Cmd+Shift+K, but the implementation in menu_builder.mm uses Ctrl+Shift+J (and Ctrl+Shift+K is already taken by Delete Line). Either update the spec to match the actual shortcut, or adjust the menu binding to align with this document.
| | Open Folder... | Cmd+Shift+K | NSOpenPanel directory mode → set File Browser root | | |
| | Open Folder... | Cmd+Shift+J | NSOpenPanel directory mode → set File Browser root | |
| if (root && [fullPath hasPrefix:root]) | ||
| { | ||
| NSString* relative = [fullPath substringFromIndex:root.length]; | ||
| if ([relative hasPrefix:@"/"]) | ||
| relative = [relative substringFromIndex:1]; | ||
| return relative; |
There was a problem hiding this comment.
The workspace-relative path check uses hasPrefix: on the raw strings. This can produce false positives when the root path is a prefix of a different directory name (e.g., /proj matches /project2/...). Consider normalizing the root to include a trailing path separator (or using -[NSString pathComponents] / URLByStandardizingPath) and verifying a path-component boundary before stripping.
| if (root && [fullPath hasPrefix:root]) | |
| { | |
| NSString* relative = [fullPath substringFromIndex:root.length]; | |
| if ([relative hasPrefix:@"/"]) | |
| relative = [relative substringFromIndex:1]; | |
| return relative; | |
| if (root.length > 0) | |
| { | |
| // Normalize root and full paths and ensure the root ends with a path separator | |
| NSString* standardizedRoot = [[root stringByStandardizingPath] stringByAppendingString:@"/"]; | |
| NSString* standardizedFullPath = [fullPath stringByStandardizingPath]; | |
| if ([standardizedFullPath hasPrefix:standardizedRoot]) | |
| { | |
| NSString* relative = [standardizedFullPath substringFromIndex:standardizedRoot.length]; | |
| if ([relative hasPrefix:@"/"]) | |
| relative = [relative substringFromIndex:1]; | |
| return relative; | |
| } |
Fixes bugs, data races, and quality issues flagged in Copilot review: - Fix data race: move tree model mutation to main queue in FSEvents handler - Fix UB: add null check for fileBrowserRootPath UTF8String load - Fix path matching: normalize root with trailing "/" before hasPrefix - Fix keyboard nav: fall back to selectedRow in both panels - Fix silent failures: add error alerts for file/folder creation - Fix modified dot: wire reloadFileSwitcherData in savepoint handlers - Fix settings save: use stringFromFileSystemPath helper - Add missing #include <memory> in file_browser_panel.mm - Implement NSTableView cell reuse in file switcher - Return full parent path for non-root files in file switcher - Remove weak stubs in panel_layout.mm, add proper includes - Remove unused kDividerThickness constant - Fix Open Folder shortcut in spec (Cmd+Shift+J not K) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 25 out of 26 changed files in this pull request and generated 10 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ^(CGFloat delta) { | ||
| ctx().leftPanelWidth += static_cast<int>(delta); | ||
| if (ctx().leftPanelWidth < static_cast<int>(kPanelCollapseThreshold)) | ||
| { | ||
| ctx().fileBrowserEnabled = false; | ||
| ctx().fileSwitcherEnabled = false; | ||
| ctx().leftPanelWidth = kDefaultLeftPanelWidth; | ||
| } |
There was a problem hiding this comment.
When the left/right zone auto-collapses during divider drag, the corresponding ctx().*Enabled flags are flipped but the menu checkmarks are not updated. This can leave the View menu showing stale checked state until the next WM_INITMENUPOPUP. Consider updating the menu state immediately on collapse/reset (e.g., call the same CheckMenuItem logic used in wndproc.mm, or centralize a helper to sync menu items from ctx()).
| - (void)closeDocument:(id)sender | ||
| { | ||
| NSInteger row = sTableView ? sTableView.clickedRow : -1; | ||
| ResolvedRow resolved; | ||
| if (!resolveRow(row, resolved)) | ||
| return; |
There was a problem hiding this comment.
Context menu actions rely solely on clickedRow. If the menu is invoked when clickedRow == -1 (e.g., keyboard invocation or programmatic menu), actions become no-ops. Consider falling back to selectedRow (as done in tableViewClicked:) for closeDocument:, saveDocument:, copyPath:, and revealInFinder:.
| // Update cell content (subviews added in fixed order: 0=modDot, 1=title, 2=path) | ||
| NSView* modDot = cell.subviews[0]; | ||
| NSTextField* titleLabel = (NSTextField*)cell.subviews[1]; | ||
| NSTextField* pathLabel = (NSTextField*)cell.subviews[2]; |
There was a problem hiding this comment.
The cell update logic assumes cell.subviews[0..2] correspond to modDot/title/path. Relying on subview order is brittle (future refactors or framework-added subviews can break it). Prefer identifying these subviews via tags/properties to make the view reuse path robust.
| [err runModal]; | ||
| } |
There was a problem hiding this comment.
After successfully creating a new file, the outline view/model isn't refreshed, so the new entry may not appear until the next FSEvents debounce/latency. Consider invalidating/reloading the affected directory (or the whole tree) immediately on success to provide deterministic UI feedback.
| [err runModal]; | |
| } | |
| [err runModal]; | |
| return; | |
| } | |
| // Immediately refresh the outline view so the newly created file appears | |
| if (sOutlineView) | |
| { | |
| [sOutlineView reloadItem:nil reloadChildren:YES]; | |
| } |
| err.informativeText = [NSString stringWithFormat:@"Could not create folder '%@'.", name]; | ||
| } | ||
| [err runModal]; | ||
| } |
There was a problem hiding this comment.
After successfully creating a new folder, the outline view/model isn't refreshed, so the new entry may not appear until the next FSEvents debounce/latency. Consider invalidating/reloading the affected directory immediately on success.
| } | |
| } | |
| else | |
| { | |
| // Immediately refresh the outline view so the new folder appears without waiting for FSEvents. | |
| if (sOutlineView) | |
| { | |
| [sOutlineView reloadItem:nil reloadChildren:YES]; | |
| } | |
| } |
| { | ||
| NSAlert* err = [NSAlert alertWithError:error]; | ||
| [err runModal]; | ||
| } |
There was a problem hiding this comment.
After a successful rename (moveItemAtPath:toPath: with no error), the tree/model isn't refreshed, so the renamed item may not update in the outline view until FSEvents fires. Consider updating the in-memory node + reloading the item (or invalidating/reloading the tree) on success.
| } | |
| } | |
| else | |
| { | |
| // Update in-memory model and refresh outline view immediately | |
| node->filename = [newName UTF8String]; | |
| node->path = [newPath UTF8String]; | |
| if (sOutlineView) | |
| { | |
| [sOutlineView reloadItem:wrapper]; | |
| } | |
| } |
macos/platform/file_browser_panel.mm
Outdated
| return; | ||
|
|
||
| NSURL* url = [NSURL fileURLWithPath:path]; | ||
| [[NSWorkspace sharedWorkspace] recycleURLs:@[url] completionHandler:nil]; |
There was a problem hiding this comment.
After moving an item to Trash, the tree/model isn't refreshed, so the deleted item can remain visible until FSEvents fires. Consider invalidating/reloading the affected directory/tree after the recycle operation completes (or after issuing it) to keep UI consistent.
| [[NSWorkspace sharedWorkspace] recycleURLs:@[url] completionHandler:nil]; | |
| [[NSWorkspace sharedWorkspace] recycleURLs:@[url] | |
| completionHandler:^(NSDictionary<NSURL*, NSURL*>* _Nonnull newURLs, NSError* _Nullable error) { | |
| dispatch_async(dispatch_get_main_queue(), ^{ | |
| // Refresh the outline view so the deleted item disappears immediately, | |
| // instead of waiting for FSEvents to update the model. | |
| if (sOutlineView) { | |
| [sOutlineView reloadData]; | |
| } | |
| }); | |
| }]; |
macos/platform/file_browser_panel.mm
Outdated
| openFolderInFileBrowser(path); | ||
|
|
||
| // Auto-enable the file browser if it is not already enabled | ||
| if (!ctx().fileBrowserEnabled) | ||
| setFileBrowserEnabled(true); |
There was a problem hiding this comment.
openFolderWithDialog() calls openFolderInFileBrowser() before ensuring the File Browser panel is enabled/initialized. If the panel is currently disabled, this can start FSEvents monitoring and set state without UI, and then setFileBrowserEnabled(true) may cause the folder to be opened twice during initialization. Consider enabling/initializing first, then opening the folder (or make openFolderInFileBrowser() a no-op until UI exists).
| openFolderInFileBrowser(path); | |
| // Auto-enable the file browser if it is not already enabled | |
| if (!ctx().fileBrowserEnabled) | |
| setFileBrowserEnabled(true); | |
| // Auto-enable the file browser if it is not already enabled | |
| if (!ctx().fileBrowserEnabled) | |
| setFileBrowserEnabled(true); | |
| openFolderInFileBrowser(path); |
macos/platform/settings_manager.mm
Outdated
| if (rootPath) | ||
| settings.fileBrowserRootPath = rootPath; | ||
| } | ||
| if ([json[@"rightPanelWidth"] isKindOfClass:[NSNumber class]]) settings.rightPanelWidth = [json[@"rightPanelWidth"] intValue]; |
There was a problem hiding this comment.
Settings loading no longer reads legacy functionListWidth / clipboardHistoryWidth keys (now replaced by rightPanelWidth). This will drop user-customized widths when upgrading from an older settings.json. Consider a small migration: if rightPanelWidth is missing, derive it from the legacy keys (e.g., max of the two) to preserve existing user preferences.
| if ([json[@"rightPanelWidth"] isKindOfClass:[NSNumber class]]) settings.rightPanelWidth = [json[@"rightPanelWidth"] intValue]; | |
| NSNumber* rightPanelWidthValue = json[@"rightPanelWidth"]; | |
| if ([rightPanelWidthValue isKindOfClass:[NSNumber class]]) | |
| { | |
| settings.rightPanelWidth = [rightPanelWidthValue intValue]; | |
| } | |
| else | |
| { | |
| // Migration from legacy width keys: derive rightPanelWidth from functionListWidth / clipboardHistoryWidth. | |
| NSNumber* functionListWidthValue = json[@"functionListWidth"]; | |
| NSNumber* clipboardHistoryWidthValue = json[@"clipboardHistoryWidth"]; | |
| int migratedWidth = 0; | |
| BOOL hasMigratedWidth = NO; | |
| if ([functionListWidthValue isKindOfClass:[NSNumber class]]) | |
| { | |
| migratedWidth = [functionListWidthValue intValue]; | |
| hasMigratedWidth = YES; | |
| } | |
| if ([clipboardHistoryWidthValue isKindOfClass:[NSNumber class]]) | |
| { | |
| int clipboardWidth = [clipboardHistoryWidthValue intValue]; | |
| if (!hasMigratedWidth || clipboardWidth > migratedWidth) | |
| { | |
| migratedWidth = clipboardWidth; | |
| hasMigratedWidth = YES; | |
| } | |
| } | |
| if (hasMigratedWidth) | |
| settings.rightPanelWidth = migratedWidth; | |
| } |
| | File | What Changes | | ||
| |------|-------------| | ||
| | `app_state.h` | Add left/right zone fields to AppContext | | ||
| | `npp_constants.h` | Add IDM_VIEW_FILEBROWSER (42088), IDM_VIEW_FILESWITCHER (42089), IDM_FILE_OPENFOLDER (42090) | |
There was a problem hiding this comment.
This plan doc states IDM_FILE_OPENFOLDER (42090), but npp_constants.h defines IDM_FILE_OPENFOLDER as 42094 (42090 is already used by IDM_FILE_REVEAL_FINDER). Please update the plan to match the actual constant to avoid future confusion during maintenance.
| | `npp_constants.h` | Add IDM_VIEW_FILEBROWSER (42088), IDM_VIEW_FILESWITCHER (42089), IDM_FILE_OPENFOLDER (42090) | | |
| | `npp_constants.h` | Add IDM_VIEW_FILEBROWSER (42088), IDM_VIEW_FILESWITCHER (42089), IDM_FILE_OPENFOLDER (42094) | |
Replaces brittle cell.subviews[N] positional indexing with NSUserInterfaceItemIdentifier-based lookup to prevent breakage from subview reordering or framework-inserted internal subviews. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
clickedRow is -1 for keyboard-invoked menus. All context menu actions and menuNeedsUpdate now use resolveTargetRow() helper. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
After creating files/folders, renaming, or trashing, the outline view now reloads immediately instead of waiting for FSEvents. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
openFolderWithDialog() now initializes the panel before calling openFolderInFileBrowser() to avoid FSEvents without UI. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
When dragging a divider past the collapse threshold, the panel enabled flags are cleared — menu checkmarks now update to match. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…anelWidth Preserves user-customized panel widths when upgrading from settings that used per-panel width keys. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Without invalidating childrenLoaded, the lazy-loading data source returns stale cached children and new items don't appear. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 25 out of 26 changed files in this pull request and generated 7 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
macos/platform/file_browser_panel.mm
Outdated
| if (sDebounceTimer) | ||
| { | ||
| dispatch_source_cancel(sDebounceTimer); | ||
| sDebounceTimer = nil; | ||
| } |
There was a problem hiding this comment.
stopFSEventsMonitoring() cancels and nils sDebounceTimer directly, but sDebounceTimer is also created/cancelled on fileBrowserQueue() in fsEventsCallback(). This shared mutable state is accessed from multiple threads without synchronization and can race/crash. Consider confining all sDebounceTimer access (create/cancel/nil) to fileBrowserQueue() (e.g., dispatch_sync/async) or protecting it with an atomic/lock.
| if (sDebounceTimer) | |
| { | |
| dispatch_source_cancel(sDebounceTimer); | |
| sDebounceTimer = nil; | |
| } | |
| dispatch_async(fileBrowserQueue(), ^{ | |
| if (sDebounceTimer) | |
| { | |
| dispatch_source_cancel(sDebounceTimer); | |
| sDebounceTimer = nil; | |
| } | |
| }); |
macos/platform/file_browser_panel.mm
Outdated
| // Update in-memory model and refresh outline view immediately | ||
| node->filename = [newName UTF8String]; | ||
| node->path = [newPath UTF8String]; | ||
| if (sOutlineView) | ||
| [sOutlineView reloadItem:wrapper]; |
There was a problem hiding this comment.
After a rename, the in-memory model only updates node->filename and node->path. If a directory is renamed and it already has loaded children, all descendant paths remain stale, so subsequent opens/copy-paths can target non-existent locations. Consider invalidating/reloading that subtree (or recursively updating descendant paths) after a successful directory rename.
| dispatch_async(dispatch_get_main_queue(), ^{ | ||
| if (sOutlineView) | ||
| [sOutlineView reloadData]; |
There was a problem hiding this comment.
After moving an item to Trash, the outline view reloads but the backing tree model isn't invalidated/updated, so deleted entries can remain visible until an FSEvents refresh happens (and FSEvents may be disabled). Consider calling invalidateChildrenRecursive(sRootNode) (or removing the node from its parent's children) before reloading, and also handling the error parameter from recycleURLs to avoid silent failures.
| dispatch_async(dispatch_get_main_queue(), ^{ | |
| if (sOutlineView) | |
| [sOutlineView reloadData]; | |
| if (error) { | |
| dispatch_async(dispatch_get_main_queue(), ^{ | |
| NSAlert* errorAlert = [[NSAlert alloc] init]; | |
| errorAlert.messageText = @"Error"; | |
| errorAlert.informativeText = [NSString stringWithFormat:@"Could not move '%@' to Trash.\n%@", name, [error localizedDescription]]; | |
| errorAlert.alertStyle = NSAlertStyleWarning; | |
| [errorAlert addButtonWithTitle:@"OK"]; | |
| [errorAlert runModal]; | |
| }); | |
| return; | |
| } | |
| dispatch_async(fileBrowserQueue(), ^{ | |
| if (sRootNode) { | |
| invalidateChildrenRecursive(sRootNode); | |
| } | |
| dispatch_async(dispatch_get_main_queue(), ^{ | |
| if (sOutlineView) | |
| [sOutlineView reloadData]; | |
| }); |
| clearSmartHighlight(ctx().scintillaView2); | ||
| ctx().activeView = 0; | ||
| bindDocumentMapToActiveView(); | ||
| bindFunctionListToActiveView(); |
There was a problem hiding this comment.
When the active editor view changes via SCN_FOCUSIN, ctx().activeView is updated but the File Switcher selection/bold state isn't refreshed. This can leave the File Switcher highlighting the wrong view until a tab switch occurs. Consider calling bindFileSwitcherToActiveView() here (similar to bindDocumentMapToActiveView / bindFunctionListToActiveView).
| bindFunctionListToActiveView(); | |
| bindFunctionListToActiveView(); | |
| bindFileSwitcherToActiveView(); |
| clearSmartHighlight(ctx().scintillaView); | ||
| ctx().activeView = 1; | ||
| bindDocumentMapToActiveView(); | ||
| bindFunctionListToActiveView(); |
There was a problem hiding this comment.
When the user focuses the split editor (SCN_FOCUSIN), ctx().activeView is set to 1 but the File Switcher isn't rebound, so its active-row highlight can become stale when switching focus between split panes without changing tabs. Consider adding bindFileSwitcherToActiveView() after updating ctx().activeView.
| bindFunctionListToActiveView(); | |
| bindFunctionListToActiveView(); | |
| bindFileSwitcherToActiveView(); |
macos/platform/settings_manager.mm
Outdated
| if (migrated > 0) | ||
| settings.rightPanelWidth = migrated; | ||
| } | ||
| if ([json[@"functionListHeightRatio"] isKindOfClass:[NSNumber class]]) settings.functionListHeightRatio = [json[@"functionListHeightRatio"] doubleValue]; |
There was a problem hiding this comment.
functionListHeightRatio is loaded from JSON without validation. If the settings file is edited/corrupted, values outside [0,1] will cause relayoutPanels() to compute invalid heights. Consider clamping to the same bounds used during divider drags (e.g., 0.15–0.85).
| if ([json[@"functionListHeightRatio"] isKindOfClass:[NSNumber class]]) settings.functionListHeightRatio = [json[@"functionListHeightRatio"] doubleValue]; | |
| if ([json[@"functionListHeightRatio"] isKindOfClass:[NSNumber class]]) | |
| { | |
| double ratio = [json[@"functionListHeightRatio"] doubleValue]; | |
| if (ratio < 0.15) | |
| ratio = 0.15; | |
| else if (ratio > 0.85) | |
| ratio = 0.85; | |
| settings.functionListHeightRatio = ratio; | |
| } |
macos/platform/file_browser_panel.mm
Outdated
| if (cacheKey.length == 0) cacheKey = @"__noext__"; | ||
| NSImage* cached = [iconCache() objectForKey:cacheKey]; | ||
| if (cached) return cached; | ||
| NSImage* icon = [[NSWorkspace sharedWorkspace] iconForFile:path]; |
There was a problem hiding this comment.
cachedIconForPath() calls -[NSWorkspace iconForFile:] synchronously on cache miss. When expanding folders with many unique extensions, this can cause noticeable UI stalls on the main thread. Consider returning a cheap placeholder icon immediately and populating the cache asynchronously (then refreshing visible rows), or using iconForFileType/UTType-based lookup to reduce filesystem work.
| NSImage* icon = [[NSWorkspace sharedWorkspace] iconForFile:path]; | |
| NSImage* icon = nil; | |
| if (isDirectory) | |
| { | |
| icon = [[NSWorkspace sharedWorkspace] iconForFileType:NSFileTypeForHFSTypeCode(kGenericFolderIcon)]; | |
| } | |
| else | |
| { | |
| NSString* ext = [path pathExtension]; | |
| if (ext.length > 0) | |
| { | |
| icon = [[NSWorkspace sharedWorkspace] iconForFileType:ext]; | |
| } | |
| else | |
| { | |
| icon = [[NSWorkspace sharedWorkspace] iconForFileType:NSFileTypeForHFSTypeCode(kGenericDocumentIcon)]; | |
| } | |
| } |
stopFSEventsMonitoring() now dispatches timer cancellation to the serial queue where the timer is created, eliminating a data race. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
When a directory is renamed, all descendant nodes retain stale paths. Now invalidates the subtree so children reload from disk. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The recycleURLs completion handler now reports errors and invalidates the model cache before reloading the outline view. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Adds bindFileSwitcherToActiveView() to SCN_FOCUSIN handlers in both main and split views so the active-row highlight stays current. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
fileBrowserHeightRatio and functionListHeightRatio are now clamped to [0.15, 0.85] on load, matching the divider drag constraints. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
cachedIconForPath now uses iconForFileType: (extension or HFS type) instead of iconForFile: to avoid synchronous filesystem access. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary:
10 commits, 23 files changed, ~2,300 lines added. Closes #23, closes #35.