Sprint: Data safety, Finder integration, app polish (6 P0 issues)#59
Merged
hybridmachine merged 3 commits intomacos-portfrom Mar 18, 2026
Merged
Sprint: Data safety, Finder integration, app polish (6 P0 issues)#59hybridmachine merged 3 commits intomacos-portfrom
hybridmachine merged 3 commits intomacos-portfrom
Conversation
Six P0 issues from the MacNotePP 1.0 project board: - #11: Tab modified indicator (bullet on dirty tabs, macOS window edited dot) - #8: Save prompt on close/quit (NSAlert with Save/Don't Save/Cancel) - #16: Handle file open from Finder (application:openFiles:, CLI args, dedup) - #12: Multi-file open support (OFN_ALLOWMULTISELECT, null-delimited paths) - #17: Proper .icns app icon and bundle metadata (sips+iconutil pipeline) - #10: About dialog with version, credits, clickable repo link New files: save_prompt.h/mm, about_dialog.h/mm Modified: 12 existing files across platform/, shim/, and CMakeLists.txt Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This was referenced Mar 17, 2026
Closed
There was a problem hiding this comment.
Pull request overview
This PR enhances the macOS port’s UX around document lifecycle and app integration by adding save-before-close prompting, modified-state indicators, multi-select file open support, a Help→About dialog, and improved app icon bundling.
Changes:
- Add save-before-close / quit prompting and hook it into close/quit paths.
- Track document modified state via Scintilla savepoint notifications and reflect it in tabs/window.
- Improve macOS integration: Help→About menu/dialog, multi-file Open, and generated
.icnsapp icon + Info.plist updates.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| macos/shim/src/win32_menu.mm | Updates GetOpenFileNameW shim to support multi-select (returns multiple paths). |
| macos/platform/file_operations.mm | Adds “switch to already-open file”, enables multi-select open parsing, updates save to clear modified state/UI. |
| macos/platform/file_operations.h | Exposes switchToFileIfOpen. |
| macos/platform/document_manager.mm | Updates window edited indicator on tab switches/closes; adds tab/window modified UI helpers. |
| macos/platform/document_manager.h | Declares updateTabModifiedIndicator / updateWindowDocumentEdited. |
| macos/platform/app_delegate.mm | Hooks Scintilla savepoint notifications to modified UI; adds openFiles handler, CLI arg open, and quit prompting hooks. |
| macos/platform/split_view.mm | Adds modified-state handling for the split view Scintilla notifications. |
| macos/platform/save_prompt.mm | New implementation for save prompts on close/close-all/quit. |
| macos/platform/save_prompt.h | New API for save prompt helpers. |
| macos/platform/wndproc.mm | Wires menu commands to new close prompting and About dialog; adds WM_CLOSE quit prompt. |
| macos/platform/menu_builder.mm | Adds Help menu + About command. |
| macos/platform/about_dialog.mm | New About dialog implementation. |
| macos/platform/about_dialog.h | Declares showAboutDlg(). |
| macos/platform/npp_constants.h | Adds Help/About command ID and Scintilla savepoint notification codes. |
| macos/platform/MacNoteInfo.plist.in | Updates bundle icon name and adds display name, min OS, category, copyright. |
| macos/CMakeLists.txt | Adds .icns generation from logo.png and packages it into the app. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+445
to
+453
| - (BOOL)windowShouldClose:(NSWindow*)sender | ||
| { | ||
| return promptAndHandleQuit() ? YES : NO; | ||
| } | ||
|
|
||
| - (NSApplicationTerminateReply)applicationShouldTerminate:(NSApplication*)sender | ||
| { | ||
| if (promptAndHandleQuit()) | ||
| return NSTerminateNow; |
Comment on lines
+373
to
+374
| if (!promptAndHandleQuit()) | ||
| return 0; |
Comment on lines
+50
to
+59
| // Untitled buffers with content are dirty even if not "modified" | ||
| if (docs[tabIndex].filePath.empty() && !docs[tabIndex].content.empty() && !docs[tabIndex].modified) | ||
| { | ||
| // Check if there's actually content in Scintilla | ||
| if (tabIndex == activeTab && sci) | ||
| { | ||
| intptr_t len = ScintillaBridge_sendMessage(sci, SCI_GETTEXTLENGTH, 0, 0); | ||
| if (len > 0) | ||
| return true; | ||
| } |
Comment on lines
+67
to
+109
| int origView = ctx().activeView; | ||
| int origTab = ctx().activeTabIndex(); | ||
|
|
||
| // Switch to the target view/tab if not already active | ||
| bool needSwitch = (viewIndex != origView || tabIndex != ctx().activeTabIndex()); | ||
| if (needSwitch) | ||
| { | ||
| ctx().activeView = viewIndex; | ||
| if (viewIndex == 0) | ||
| { | ||
| if (tabIndex != ctx().activeTab) | ||
| switchToTabInView(0, tabIndex); | ||
| } | ||
| else | ||
| { | ||
| if (tabIndex != ctx().activeTab2) | ||
| switchToTabInView(1, tabIndex); | ||
| } | ||
| } | ||
|
|
||
| auto& docs = ctx().activeDocuments(); | ||
| std::wstring pathBefore = docs[ctx().activeTabIndex()].filePath; | ||
|
|
||
| saveCurrentFile(); | ||
|
|
||
| auto& docsAfter = ctx().activeDocuments(); | ||
| int tabIdx = ctx().activeTabIndex(); | ||
| bool saved = false; | ||
| if (tabIdx >= 0 && tabIdx < static_cast<int>(docsAfter.size())) | ||
| { | ||
| // Save succeeded if: file has a path AND is no longer modified | ||
| void* sci = ctx().activeScintillaView(); | ||
| saved = !docsAfter[tabIdx].filePath.empty() && | ||
| (sci ? ScintillaBridge_sendMessage(sci, SCI_GETMODIFY, 0, 0) == 0 : !docsAfter[tabIdx].modified); | ||
| } | ||
|
|
||
| // Restore original context if we switched | ||
| if (needSwitch) | ||
| { | ||
| ctx().activeView = origView; | ||
| } | ||
|
|
||
| return saved; |
Comment on lines
+136
to
+142
| // Snapshot dirty tab indices (iterate in reverse to avoid index shifting issues) | ||
| std::vector<int> dirtyTabs; | ||
| for (int i = 0; i < static_cast<int>(docs.size()); ++i) | ||
| { | ||
| if (isDocumentDirty(viewIndex, i)) | ||
| dirtyTabs.push_back(i); | ||
| } |
| // Canonicalize the path for comparison | ||
| NSString* nsPath = WideToNSString(filePath.c_str()); | ||
| NSString* canonical = [[nsPath stringByStandardizingPath] stringByResolvingSymlinksInPath]; | ||
| std::wstring canonicalPath = NSStringToWide(canonical); |
| COMMAND sips -z 256 256 "${NPP_ROOT}/logo.png" --out "${ICONSET_DIR}/icon_256x256.png" | ||
| COMMAND sips -z 512 512 "${NPP_ROOT}/logo.png" --out "${ICONSET_DIR}/icon_256x256@2x.png" | ||
| COMMAND sips -z 512 512 "${NPP_ROOT}/logo.png" --out "${ICONSET_DIR}/icon_512x512.png" | ||
| COMMAND sips -z 583 583 "${NPP_ROOT}/logo.png" --out "${ICONSET_DIR}/icon_512x512@2x.png" |
Keep save-point notifications suppressed through programmatic Scintilla state restores/new-tab initialization so SCN_SAVEPOINTREACHED cannot incorrectly clear dirty state. Also persist save-point validity tracking and related guard handling. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…hang Add CFBundleDocumentTypes to Info.plist so MacNote++ appears in Finder's "Open With" menu for text, source code, XML, JSON, and shell script files. Fix About dialog becoming unresponsive when dismissed via Escape — the modal run loop was never stopped because only the OK button called stopModal. Add windowWillClose: delegate to end the modal session regardless of how the panel is closed. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements all 6 Ready items from the MacNotePP 1.0 project board — the first sprint of P0 core-polish work.
•on dirty tabs, macOS window-edited dot viaSCN_SAVEPOINTLEFT/REACHEDapplication:openFiles:for double-click/Open With/Dock drag; CLI arg handling; duplicate-tab prevention with canonical path comparisonOFN_ALLOWMULTISELECTin open dialog; shim returns null-delimited full paths viapanel.URLs.icnsgeneration via sips+iconutil CMake pipeline; updated Info.plist withLSMinimumSystemVersion,LSApplicationCategoryType, copyrightNew files
save_prompt.h/mmabout_dialog.h/mmModified files (12)
app_delegate.mm,document_manager.h/mm,file_operations.h/mm,menu_builder.mm,npp_constants.h,split_view.mm,wndproc.mm,win32_menu.mm,CMakeLists.txt,MacNoteInfo.plist.inTest plan
filename •and window close button shows dotopen -a MacNote++ file.txtfrom Terminal → file opens🤖 Generated with Claude Code
Closes #8, closes #10, closes #11, closes #12, closes #16, closes #17