fix: restore macOS quit/close shortcuts#59
Conversation
📝 WalkthroughWalkthroughReplaced a static Electron application menu with platform-aware construction: when Changes
Sequence Diagram(s)(omitted — changes are localized menu construction, not multi-component sequential flow) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Tip You can disable the changed files summary in the walkthrough.Disable the |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
.github/workflows/build-desktop.yml (2)
360-366: Non-macOS platforms may benefit from a File menu with Exit option.The Windows/Linux menu lacks a File menu with an "Exit" option. While closing via
Alt+F4or the window close button works, many users expect a File → Exit menu option on these platforms for discoverability.♻️ Optional: Add File menu for Windows/Linux
] : [\n' + + {\n' + + label: \'File\',\n' + + submenu: [\n' + + { role: \'quit\' }\n' + + ]\n' + + },\n' + {\n' + label: \'Edit\',\n' +🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/build-desktop.yml around lines 360 - 366, Add a "File" menu entry for non-macOS platforms by inserting a menu object with label 'File' and a submenu containing an Exit item (use role: 'quit' or a click that calls app.quit()) alongside the existing 'Window' menu block; ensure this menu is only added when process.platform !== 'darwin' so macOS behavior is unchanged and use the existing menu construction area around the 'Window' menu (look for label: 'Window' and submenu roles 'minimize'/'close') to place the new File → Exit item.
326-332: Consider using thewindowMenurole for a more complete Window menu on macOS.The current Window menu only includes
minimizeandclose. On macOS, the standard Window menu typically also includes items like "Zoom", "Bring All to Front", and a list of open windows. Electron'swindowMenurole provides this automatically.♻️ Alternative using built-in windowMenu role
{\n' + - label: \'Window\',\n' + - submenu: [\n' + - { role: \'minimize\' },\n' + - { role: \'close\' }\n' + - ]\n' + + role: \'windowMenu\'\n' + }\n' +🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/build-desktop.yml around lines 326 - 332, Replace the custom "Window" menu object (the menu entry with label 'Window' and submenu containing { role: 'minimize' } and { role: 'close' }) with Electron's built-in window menu by using the role 'windowMenu'; locate the menu template where the "Window" label and its submenu are defined and change it to use role: 'windowMenu' (or, if keeping a custom entry, add the missing standard items such as 'zoom' and 'front' and let Electron populate window lists) so macOS gets the complete standard Window menu behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/build-desktop.yml:
- Line 284: The string literal fragment "' ' // 提供稳定的菜单与编辑快捷键（生产环境）"
prematurely closes the JS string and breaks concatenation; change that fragment
so the literal includes the newline escape (i.e. add \n inside the quotes) and
keep the '+' concatenation operator in place, moving the inline comment outside
the string (place the comment after the '+' or otherwise outside the quoted
text) so the generated JavaScript becomes a valid concatenated string.
---
Nitpick comments:
In @.github/workflows/build-desktop.yml:
- Around line 360-366: Add a "File" menu entry for non-macOS platforms by
inserting a menu object with label 'File' and a submenu containing an Exit item
(use role: 'quit' or a click that calls app.quit()) alongside the existing
'Window' menu block; ensure this menu is only added when process.platform !==
'darwin' so macOS behavior is unchanged and use the existing menu construction
area around the 'Window' menu (look for label: 'Window' and submenu roles
'minimize'/'close') to place the new File → Exit item.
- Around line 326-332: Replace the custom "Window" menu object (the menu entry
with label 'Window' and submenu containing { role: 'minimize' } and { role:
'close' }) with Electron's built-in window menu by using the role 'windowMenu';
locate the menu template where the "Window" label and its submenu are defined
and change it to use role: 'windowMenu' (or, if keeping a custom entry, add the
missing standard items such as 'zoom' and 'front' and let Electron populate
window lists) so macOS gets the complete standard Window menu behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 69021f67-b0ce-4407-9253-bedfea51ec6b
📒 Files selected for processing (1)
.github/workflows/build-desktop.yml
.github/workflows/build-desktop.yml
Outdated
| ' }\\n' + | ||
| ' ];\\n' + | ||
| ' Menu.setApplicationMenu(Menu.buildFromTemplate(menuTemplate));\\n\\n' + | ||
| ' ' // 提供稳定的菜单与编辑快捷键（生产环境）\n' + |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check line 284 for proper string syntax
sed -n '284p' .github/workflows/build-desktop.yml
# Check if the node script in the workflow has balanced quotes
awk '/Create Electron main process/,/fs.writeFileSync\(.*main.js/' .github/workflows/build-desktop.yml | grep -n "'"Repository: AmintaCCCP/GithubStarsManager
Length of output: 11022
🏁 Script executed:
#!/bin/bash
# Get the full context around line 284
sed -n '280,290p' .github/workflows/build-desktop.yml | cat -n
# Also check the lines around it in the original file to understand the structure
echo "=== Lines 275-295 ==="
sed -n '275,295p' .github/workflows/build-desktop.yml | cat -nRepository: AmintaCCCP/GithubStarsManager
Length of output: 1909
🏁 Script executed:
#!/bin/bash
# Check if there are any workflow run logs or recent commits related to this file
git log --oneline -10 .github/workflows/build-desktop.yml
# Also check if the file syntax is validated by the workflow itself
echo "=== Checking for Node/JS validation in workflow ==="
grep -n "node\|javascript\|eslint" .github/workflows/build-desktop.yml | head -20Repository: AmintaCCCP/GithubStarsManager
Length of output: 585
🏁 Script executed:
#!/bin/bash
# Check the actual workflow execution - does it build successfully despite the syntax error?
# Get commits around this file
git log --format="%H %s" -5 .github/workflows/build-desktop.yml
# Check if there are workflow run status indicators
echo "=== Checking workflow syntax with Node ==="
# Try to simulate what the workflow does - extract and validate the JavaScript string
sed -n '12,370p' .github/workflows/build-desktop.yml | grep -A 5 "mainJsContent = " | head -20Repository: AmintaCCCP/GithubStarsManager
Length of output: 586
Critical syntax error: string concatenation broken by comment syntax.
Line 284 prematurely terminates the string literal with ' ' and then attempts a comment // ..., which breaks the concatenation chain. The + operator after the comment would be unreachable, causing the string concatenation to fail. The generated JavaScript code would be invalid.
This line should be: ' \\n' + // 提供稳定的菜单与编辑快捷键（生产环境）
The comment belongs in the YAML/workflow file, not in the generated JavaScript string.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/build-desktop.yml at line 284, The string literal fragment
"' ' // 提供稳定的菜单与编辑快捷键（生产环境）" prematurely closes the JS string and
breaks concatenation; change that fragment so the literal includes the newline
escape (i.e. add \n inside the quotes) and keep the '+' concatenation operator
in place, moving the inline comment outside the string (place the comment after
the '+' or otherwise outside the quoted text) so the generated JavaScript
becomes a valid concatenated string.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
.github/workflows/build-desktop.yml (1)
284-284:⚠️ Potential issue | 🔴 CriticalFix broken string concatenation in generated
mainJsContent(syntax blocker).Line [284] has invalid adjacent string literals (
'...' '...') without+, which breaks the Node script generatingelectron/main.js.🛠️ Proposed fix
- ' ' ' // 提供稳定的菜单与编辑快捷键（生产环境）\n' + + ' // 提供稳定的菜单与编辑快捷键（生产环境）\\n' +🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/build-desktop.yml at line 284, The generated mainJsContent contains invalid adjacent string literals (e.g., "'...' '...'") which breaks the Node script that writes electron/main.js; locate the code that builds mainJsContent (the variable constructing the concatenated JS string) and fix the concatenation by inserting the missing + operators (or merging the literals into one string) so each piece is properly joined, then run the generator to verify electron/main.js is syntactically valid.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In @.github/workflows/build-desktop.yml:
- Line 284: The generated mainJsContent contains invalid adjacent string
literals (e.g., "'...' '...'") which breaks the Node script that writes
electron/main.js; locate the code that builds mainJsContent (the variable
constructing the concatenated JS string) and fix the concatenation by inserting
the missing + operators (or merging the literals into one string) so each piece
is properly joined, then run the generator to verify electron/main.js is
syntactically valid.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4f9cfe48-adb8-43b5-8e22-1fe1270c0a90
📒 Files selected for processing (1)
.github/workflows/build-desktop.yml
There was a problem hiding this comment.
♻️ Duplicate comments (1)
.github/workflows/build-desktop.yml (1)
284-369:⚠️ Potential issue | 🔴 CriticalUnescaped
"in thenode -epayload breaks the workflow script.These lines use raw double-quoted JS fragments inside a shell double-quoted
node -e "..."block (started at Line 182). The shell string is terminated early, so this step fails before writingelectron/main.js.🐛 Minimal fix direction
- " // 提供稳定的菜单与编辑快捷键（生产环境）\\n" + - " const menuTemplate = process.platform === 'darwin' ? [\\n" + + ' // 提供稳定的菜单与编辑快捷键（生产环境）\\n' + + ' const menuTemplate = process.platform === \\'darwin\\' ? [\\n' +Apply the same quoting style consistently through this inserted block (Lines 284-369), or escape every internal
"as\".#!/bin/bash # Verify problematic raw double-quoted lines inside the node -e block. sed -n '182,414p' .github/workflows/build-desktop.yml | nl -ba | sed -n '95,210p' echo "---- raw double-quoted fragments inside node -e payload ----" sed -n '182,414p' .github/workflows/build-desktop.yml | rg -n '^[[:space:]]*"'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/build-desktop.yml around lines 284 - 369, The node -e payload contains unescaped double quotes inside the JS fragment (the menuTemplate string and its entries) which prematurely terminates the shell double-quoted string; fix by either escaping all internal " as \" or by changing how the payload is quoted (e.g., use a single-quoted shell string or a heredoc) so the JavaScript block including menuTemplate, Menu.buildFromTemplate and Menu.setApplicationMenu is preserved intact; locate the node -e invocation that writes electron/main.js and update the quoting for the inserted fragment accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In @.github/workflows/build-desktop.yml:
- Around line 284-369: The node -e payload contains unescaped double quotes
inside the JS fragment (the menuTemplate string and its entries) which
prematurely terminates the shell double-quoted string; fix by either escaping
all internal " as \" or by changing how the payload is quoted (e.g., use a
single-quoted shell string or a heredoc) so the JavaScript block including
menuTemplate, Menu.buildFromTemplate and Menu.setApplicationMenu is preserved
intact; locate the node -e invocation that writes electron/main.js and update
the quoting for the inserted fragment accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 46306543-6e5c-4598-bdda-3e8c8ef41346
📒 Files selected for processing (1)
.github/workflows/build-desktop.yml
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/build-desktop.yml (1)
300-367: Optional cleanup: deduplicate shared menu sections.Line 300-Line 367 repeats the same Edit/View/Window blocks in both branches. Consider extracting shared menus and only prepending the macOS app menu on darwin to reduce maintenance drift.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/build-desktop.yml around lines 300 - 367, The Edit/View/Window submenu blocks are duplicated; extract them into a single sharedMenus constant (e.g., sharedMenus containing the Edit, View, and Window objects) and then build the final menu template by conditionally prepending the macOS app menu when on darwin (e.g., menuTemplate = isMac ? [appMenu, ...sharedMenus] : sharedMenus), replacing the duplicated branches in the current template construction.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.github/workflows/build-desktop.yml:
- Around line 300-367: The Edit/View/Window submenu blocks are duplicated;
extract them into a single sharedMenus constant (e.g., sharedMenus containing
the Edit, View, and Window objects) and then build the final menu template by
conditionally prepending the macOS app menu when on darwin (e.g., menuTemplate =
isMac ? [appMenu, ...sharedMenus] : sharedMenus), replacing the duplicated
branches in the current template construction.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5d60689f-27df-409b-8385-daa98a348220
📒 Files selected for processing (1)
.github/workflows/build-desktop.yml
1 participant
Restore macOS App/Window menus in desktop build so Command+Q and Command+W work again. Fixes #58.
Summary by CodeRabbit