Clarify cli (dev only) mode #4570
Conversation
WalkthroughAdds i18n key Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant Config
participant i18n
User->>CLI: Open CLI tab
CLI->>Config: getConfig("cliOnlyMode")
Config-->>CLI: { cliOnlyMode?: boolean }
CLI->>i18n: key = cliOnlyMode ? "cliDevEnter" : "cliEnter"
i18n-->>CLI: Localized message
CLI-->>User: Display startup message
Estimated code review effort🎯 2 (Simple) | ⏱️ ~7 minutes Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used🧬 Code Graph Analysis (1)src/js/tabs/cli.js (3)
🔇 Additional comments (1)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
![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 (1)
locales/en/messages.json (1)
3394-3396: New i18n key added; consider clearer phrasingKey addition looks fine. Minor wording nit: “Developer CLI-only mode detected” reads a bit clearer and matches the hyphenation style of “CLI-only”.
Apply this optional diff to tweak the message:
- "cliDevEnter": { - "message": "CLI only developer mode detected" - }, + "cliDevEnter": { + "message": "Developer CLI-only mode detected" + },
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
locales/en/messages.json(1 hunks)src/js/tabs/cli.js(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/js/tabs/cli.js (3)
src/js/gui_log.js (1)
gui_log(7-21)src/js/localization.js (1)
i18n(7-7)src/js/model.js (1)
getConfig(228-228)
🔇 Additional comments (1)
src/js/tabs/cli.js (1)
479-481: Verify getConfig(“cliOnlyMode”) Shape and DefaultsWe’ve searched the codebase for usages of cliOnlyMode and getConfig, but didn’t find a source-level definition of getConfig. Here’s what we observed:
• In src/js/serial_backend.js:
js function isCliOnlyMode() { return getConfig("cliOnlyMode")?.cliOnlyMode === true; }
• In src/js/tabs/cli.js:
js gui_log( i18n.getMessage( getConfig("cliOnlyMode")?.cliOnlyMode ? "cliEnter" : "cliDevEnter" ) );
• In src/js/tabs/options.js:
js const result = getConfig("cliOnlyMode", false); cliOnlyModeElement.prop("checked", !!result.cliOnlyMode);
• No definition of getConfig(...) was found by ripgrep—getConfig must be defined elsewhere (e.g. in model.js or a shared util).Please manually verify:
- That getConfig takes a key and optional default value and always returns an object of the form
{ cliOnlyMode: boolean }.- That when no stored setting exists,
getConfig("cliOnlyMode")yields{ cliOnlyMode: false }(non‐dev behavior).- That the second-parameter default (
false) is being passed correctly into the underlying getConfig implementation.Once you’ve located and confirmed the implementation semantics and defaults, update or document getConfig accordingly.
haslinghuis
force-pushed
the
clarify-cli-mode
branch
from
5d81715 to
af6b5cb
Compare
|
|
Preview URL: https://af6b5cb7.betaflight-configurator.pages.dev |
Tested fine. Thanks, with this it will be easier to understand that a developer setting is enabled in options.
|
|
Not sure is influent, so just as advice, with virtual connection enabled and dev>cli mode only enabled, goes in cli but there is no message at all in log. |
| "message": "CLI mode detected" | ||
| }, | ||
| "cliDevEnter": { | ||
| "message": "CLI only developer mode detected" |
4 participants
Summary by CodeRabbit
New Features
Chores