Print version when run from CLI with --version or -v
#1559
Conversation
WalkthroughThe pull request introduces a command-line argument for version checking in the application's Changes
Possibly related PRs
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
emain/emain.ts (2)
568-576: Consider using a CLI argument parser for more robust flag handling.While the current implementation works, it has limitations:
- Only checks argv[1], missing flags if they appear later
- No error handling for getWaveVersion()
Consider using a CLI argument parsing library like
yargsorcommanderfor:
- More flexible argument position handling
- Built-in help text generation
- Consistent error handling
Example implementation:
- const args = process.argv; - if (args?.length > 1 && (args[1] === "--version" || args[1] === "-v")) { - const versionInfo = getWaveVersion(); - console.log(`Wave Terminal v${versionInfo.version} (build ${versionInfo.buildTime})`); - electronApp.quit(); - return; - } + const yargs = require('yargs/yargs'); + const argv = yargs(process.argv.slice(1)) + .version(false) // Disable default version + .option('version', { + alias: 'v', + describe: 'Show version information', + type: 'boolean' + }) + .parse(); + + if (argv.version) { + try { + const versionInfo = getWaveVersion(); + console.log(`Wave Terminal v${versionInfo.version} (build ${versionInfo.buildTime})`); + } catch (error) { + console.error('Error retrieving version information:', error); + } + electronApp.quit(); + return; + }
Line range hint
1-575: Document the rationale for ignoring QUIC protocol errors.The error handling for QUIC protocol errors lacks documentation explaining why these errors can be safely ignored.
Add a comment explaining the context:
process.on("uncaughtException", (error) => { if (caughtException) { return; } + // QUIC protocol errors can occur during auto-updates when the updater + // attempts to establish a connection. These errors are non-fatal and + // can be safely ignored as they don't affect the core functionality + // of the application. if (error?.message?.includes("net::ERR_QUIC_PROTOCOL_ERROR")) { console.log("Ignoring QUIC protocol error:", error.message); console.log("Stack Trace:", error.stack); return; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
emain/emain.ts(1 hunks)
🔇 Additional comments (1)
emain/emain.ts (1)
Line range hint 1-575: Consider more robust QUIC error detection.
The current string-based error detection might be fragile if error messages change.
Consider checking error types or codes instead:
- if (error?.message?.includes("net::ERR_QUIC_PROTOCOL_ERROR")) {
+ // Check for specific error types/codes that indicate QUIC protocol errors
+ if (error?.code === 'ERR_QUIC_PROTOCOL_ERROR' ||
+ (error instanceof Error && error.name === 'QuicError')) {Let's verify if there are other QUIC-related error patterns we should handle:
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
docs/docs/faq.mdx (2)
36-36: Improve phrase clarityThe phrase "outside of" is redundant.
-`wsh` is an internal CLI for extending control over Wave to the command line, you can learn more about it [here](./wsh). To prevent misuse by other applications, `wsh` requires an access token provided by Wave to work and will not function outside of the app. +`wsh` is an internal CLI for extending control over Wave to the command line, you can learn more about it [here](./wsh). To prevent misuse by other applications, `wsh` requires an access token provided by Wave to work and will not function outside Wave.🧰 Tools
🪛 LanguageTool
[style] ~36-~36: This phrase is redundant. Consider using “outside”.
Context: ...d by Wave to work and will not function outside of the app. ## Why does Wave warn me abou...(OUTSIDE_OF)
40-40: Fix grammatical and hyphenation issuesThere are two minor issues in this paragraph:
- The word "however" should be properly separated from the sentence
- The hyphen in "natively-compiled" is redundant
-macOS and Windows both have compatibility layers that allow x64 applications to run on ARM computers. This helps more apps run on these systems while developers work to add native ARM support to their applications, however it comes with significant performance tradeoffs. To get the best experience using Wave, it is recommended to install the version of Wave that is natively-compiled for your computer. +macOS and Windows both have compatibility layers that allow x64 applications to run on ARM computers. This helps more apps run on these systems while developers work to add native ARM support to their applications. However, it comes with significant performance tradeoffs. To get the best experience using Wave, it is recommended to install the version of Wave that is natively compiled for your computer.🧰 Tools
🪛 LanguageTool
[typographical] ~40-~40: The word “however” is an adverb that can’t be used like a conjunction, and therefore needs to be separated from the sentence.
Context: ...native ARM support to their applications, however it comes with significant performance t...(HOWEVER_SENTENCE)
[uncategorized] ~40-~40: The hyphen in natively-compiled is redundant.
Context: ... to install the version of Wave that is natively-compiled for your computer. You can find the rig...(ADVERB_LY_HYPHEN_FIX)
emain/platform.ts (1)
44-63: Consider enhancing the architecture warning implementationThe implementation is functional but could be improved for maintainability.
Consider these improvements:
+const ARCH_WARNING_DIALOG_OPTIONS = { + type: "warning", + buttons: ["See documentation", "Dismiss"], + title: "Wave has detected a performance issue", +}; + +const ARCH_WARNING_DOCS_URL = "https://docs.waveterm.dev"; + export function checkIfRunningUnderARM64Translation(fullConfig: FullConfigType) { if (!fullConfig.settings["app:dismissarchitecturewarning"] && app.runningUnderARM64Translation) { console.log("Running under ARM64 translation, alerting user"); - const dialogOpts: Electron.MessageBoxOptions = { - type: "warning", - buttons: ["See documentation", "Dismiss"], - title: "Wave has detected a performance issue", - message: `Wave has detected that it is running in ARM64 translation mode.\n\nThis may cause performance issues.\n\nPlease download the native version of Wave for your architecture (${unameArch})`, - }; + const dialogOpts: Electron.MessageBoxOptions = { + ...ARCH_WARNING_DIALOG_OPTIONS, + message: `Wave has detected that it is running in ARM64 translation mode.\n\nThis may cause performance issues.\n\nPlease download the native version of Wave for your architecture (${unameArch})`, + }; const choice = dialog.showMessageBoxSync(null, dialogOpts); if (choice === 0) { // Open the documentation URL console.log("Opening documentation URL"); - fireAndForget(() => shell.openExternal("https://docs.waveterm.dev")); + fireAndForget(() => shell.openExternal(ARCH_WARNING_DOCS_URL)); } else { console.log("User dismissed the dialog"); } } }emain/emain.ts (2)
622-629: LGTM! Consider adding error handling for package.json reading.The version flag implementation follows standard CLI patterns. However, consider adding try-catch for the require statement to handle potential file reading errors gracefully.
const args = process.argv; if (args?.length > 1 && (args.includes("--version") || args.includes("-v"))) { + try { const pkg = require("../package.json"); console.log(`Wave Terminal v${pkg.version}`); + } catch (e) { + console.error("Error reading version information:", e); + } electronApp.quit(); }
592-592: Consider adding error handling for architecture check.While the placement is correct, consider wrapping the architecture check in a try-catch to ensure any failures don't prevent the application from starting.
- checkIfRunningUnderARM64Translation(fullConfig); + try { + checkIfRunningUnderARM64Translation(fullConfig); + } catch (e) { + console.error("Error checking architecture:", e); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
Taskfile.yml(1 hunks)docs/docs/config.mdx(1 hunks)docs/docs/faq.mdx(1 hunks)docs/docs/gettingstarted.mdx(1 hunks)emain/emain.ts(3 hunks)emain/platform.ts(2 hunks)frontend/types/gotypes.d.ts(1 hunks)pkg/wconfig/metaconsts.go(1 hunks)pkg/wconfig/settingsconfig.go(1 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/docs/faq.mdx
[style] ~36-~36: This phrase is redundant. Consider using “outside”.
Context: ...d by Wave to work and will not function outside of the app. ## Why does Wave warn me abou...
(OUTSIDE_OF)
[typographical] ~40-~40: The word “however” is an adverb that can’t be used like a conjunction, and therefore needs to be separated from the sentence.
Context: ...native ARM support to their applications, however it comes with significant performance t...
(HOWEVER_SENTENCE)
[uncategorized] ~40-~40: The hyphen in natively-compiled is redundant.
Context: ... to install the version of Wave that is natively-compiled for your computer. You can find the rig...
(ADVERB_LY_HYPHEN_FIX)
🔇 Additional comments (8)
pkg/wconfig/metaconsts.go (1)
11-11: LGTM! The new config key follows conventions
The new configuration key follows the established naming pattern and is correctly placed in the App-related configuration section.
docs/docs/gettingstarted.mdx (1)
17-45: LGTM! Well-structured platform requirements section
The new platform requirements section is well-organized and provides clear information about supported architectures and OS versions for each platform.
Let's verify the cross-references and issue link:
✅ Verification successful
Issue #928 link and ARM64 support information is accurate
The documentation correctly reflects the current state of Windows ARM64 support:
- Issue Windows ARM support #928 exists and is correctly referenced
- The issue is open and actively tracked
- The blocker is accurately described (waiting for upstream dependencies)
- Linux and macOS ARM64 support is already available, as confirmed by build configurations and README
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the Windows ARM support issue link and related documentation
# Check if the Windows ARM support issue exists and is referenced correctly
gh issue view 928
# Check for any other related issues or PRs
gh search issues "ARM64 support" --repo wavetermdev/waveterm --state all
Length of output: 3265
Script:
#!/bin/bash
# Let's verify Windows ARM support related information with correct flags
# Check for open issues related to ARM64 support
gh search issues "ARM64 support in:title,body repo:wavetermdev/waveterm state:open"
# Check for closed issues related to ARM64 support
gh search issues "ARM64 support in:title,body repo:wavetermdev/waveterm state:closed"
# Let's also check the codebase for any ARM64-related build configurations or discussions
rg -i "arm64|aarch64" --type md --type mdx -C 2
Length of output: 5573
Script:
#!/bin/bash
# Let's try a different approach to verify ARM-related content
# Check for ARM-related content in markdown files
rg -i "arm64|aarch64" -t md -C 2
# Check the specific issue #928 mentioned in the docs
gh issue view 928 --json title,state,body,comments
# Look for any build or platform configuration files that might contain ARM-related settings
rg -i "arm64|aarch64" -g "*.{json,yaml,yml,toml}" -C 2
Length of output: 4815
emain/platform.ts (1)
4-5: LGTM: Clean import additions
The new imports are properly organized and necessary for the dialog and shell functionality.
Taskfile.yml (1)
99-99: LGTM: Clean addition of CLI args support
The addition of {{.CLI_ARGS}} properly enables passing version flag and other arguments to electron-builder.
pkg/wconfig/settingsconfig.go (2)
36-37: LGTM: Improved field alignment
The realignment of existing fields enhances code readability while maintaining functionality.
38-38: LGTM: Well-structured settings addition
The new AppDismissArchitectureWarning field follows the established patterns and properly implements the architecture warning dismissal functionality.
docs/docs/config.mdx (1)
29-29: LGTM! Documentation is clear and well-structured.
The new configuration key is well-documented, follows the existing format, and includes helpful references to additional information.
frontend/types/gotypes.d.ts (1)
621-621: LGTM! Type definition is correct and consistent.
The new property is properly typed as an optional boolean and follows the existing pattern in the SettingsType interface.
Summary by CodeRabbit