feat: androidEmulatorStartArgs nativescript cli option#6038
Conversation
📝 WalkthroughWalkthrough
ChangesUser-configured Android emulator start arguments
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lib/common/mobile/android/android-emulator-services.ts`:
- Around line 199-204: The try/catch around reading androidEmulatorStartArgs
silently swallows errors; modify the block that calls
this.$userSettingsService.getSettingValue<string[]>("androidEmulatorStartArgs")
(and the subsequent startEmulatorArgs.push(...additionalArgs)) to catch errors
but log them at trace level instead of ignoring them: use the service's logger
(e.g. this.$logger.trace or equivalent) to record the caught error with context
("reading androidEmulatorStartArgs") and then continue to preserve the non-fatal
behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: e522b3f1-3dda-4388-9b46-753865e50bb3
📒 Files selected for processing (1)
lib/common/mobile/android/android-emulator-services.ts
| try { | ||
| const additionalArgs = await this.$userSettingsService.getSettingValue<string[]>("androidEmulatorStartArgs"); | ||
| if (additionalArgs?.length) { | ||
| startEmulatorArgs.push(...additionalArgs); | ||
| } | ||
| } catch (error) {} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -nP -C3 'interface\s+IUserSettingsService' --type=ts
ast-grep --pattern 'getSettingValue<$_>($_): $_'Repository: NativeScript/nativescript-cli
Length of output: 464
Don’t silently swallow settings/emulator-args read errors; log at trace level.
The empty catch around androidEmulatorStartArgs retrieval (and startEmulatorArgs.push(...)) discards any failures, making misconfiguration hard to diagnose while keeping the non-fatal behavior.
♻️ Proposed change
try {
const additionalArgs = await this.$userSettingsService.getSettingValue<string[]>("androidEmulatorStartArgs");
if (additionalArgs?.length) {
startEmulatorArgs.push(...additionalArgs);
}
- } catch (error) {}
+ } catch (error) {
+ this.$logger.trace(
+ `Failed to read 'androidEmulatorStartArgs' setting. More info: ${error}`,
+ );
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| try { | |
| const additionalArgs = await this.$userSettingsService.getSettingValue<string[]>("androidEmulatorStartArgs"); | |
| if (additionalArgs?.length) { | |
| startEmulatorArgs.push(...additionalArgs); | |
| } | |
| } catch (error) {} | |
| try { | |
| const additionalArgs = await this.$userSettingsService.getSettingValue<string[]>("androidEmulatorStartArgs"); | |
| if (additionalArgs?.length) { | |
| startEmulatorArgs.push(...additionalArgs); | |
| } | |
| } catch (error) { | |
| this.$logger.trace( | |
| `Failed to read 'androidEmulatorStartArgs' setting. More info: ${error}`, | |
| ); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/common/mobile/android/android-emulator-services.ts` around lines 199 -
204, The try/catch around reading androidEmulatorStartArgs silently swallows
errors; modify the block that calls
this.$userSettingsService.getSettingValue<string[]>("androidEmulatorStartArgs")
(and the subsequent startEmulatorArgs.push(...additionalArgs)) to catch errors
but log them at trace level instead of ignoring them: use the service's logger
(e.g. this.$logger.trace or equivalent) to record the caught error with context
("reading androidEmulatorStartArgs") and then continue to preserve the non-fatal
behavior.
This simple PR allows to define custom Android emulator start arguments
Summary by CodeRabbit