feat: devtools windows support #12
Conversation
WalkthroughThis pull request adds cross-platform software discovery utilities (including GBK decoding and Windows uninstall-registry lookup) and integrates them into devtools logic to resolve the CLI path on Windows; also updates the Windows bundle ID for mp-weixin to "微信开发者工具". Changes
Sequence Diagram(s)sequenceDiagram
participant DevTools as DevTools Logic
participant Finder as findSoftwareInstallLocation
participant Registry as findInUninstallRegistry
participant PS as PowerShell
participant FS as File System
DevTools->>Finder: findSoftwareInstallLocation(executableName, displayName)
alt Windows with displayName
Finder->>Registry: findInUninstallRegistry(displayName)
Registry->>PS: execute registry query
PS-->>Registry: path (InstallLocation or DisplayIcon)
Registry-->>Finder: [path, source]
else Windows fallback
Finder->>FS: derive path from AppCompatFlags / executable name
FS-->>Finder: derived path
else macOS
Finder->>FS: check /Applications/{executableName}.app
FS-->>Finder: app path
end
Finder-->>DevTools: CLI path or null
DevTools->>FS: resolve cli.bat at discovered location
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–25 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
![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 (4)
src/logics/devtools.ts (1)
9-9: Clarify executable vs display name for Windows devtools detectionOn Windows (Line 55),
DEVTOOLS_BUNDLE_ID.windows['mp-weixin'](Line 17) is used both asexecutableNameanddisplayNamewhen callingfindSoftwareInstallLocation(Line 60). The helper, however, treats:
- first parameter as the executable base name for AppCompat (
<name>.exe), and- second as the Control Panel display name for the uninstall registry lookup.
This is only safe if the actual
.exefile name exactly matches the display name (minus the extension). To make the fallback more robust and self‑documenting, consider:
- storing separate
exeNameanddisplayNameconstants for Windows, or- passing the known executable base name as the first argument and the localized display name as the second.
Also applies to: 17-18, 55-64
src/utils/index.ts (3)
42-45: HardendecodeGbktyping and runtime compatibility
decodeGbkcurrently usesNonSharedBufferandnew TextDecoder('gbk'):
- Ensure
NonSharedBufferis actually available in your type environment; otherwise this will not type‑check. A safer, more generic signature would be something likeinput?: Uint8Array | ArrayBuffer(or simplyBuffer | Uint8Arrayfor Node).TextDecoder('gbk')will throw if the runtime’sTextDecoderdoesn’t support the'gbk'label. If you need to support a wider range of Node versions/builds, consider instantiating the decoder once in a guarded block or falling back to UTF‑8 when GBK isn’t available.Please verify your Node.js target version supports
TextDecoderwith the'gbk'label and thatNonSharedBufferis defined in your typings (e.g., by searching for atype NonSharedBufferalias or checkingtsconfig.jsontypessettings).
53-116: Align or documentfindSoftwareInstallLocationreturn shape across platformsOn Windows, this helper returns the directory that contains the executable (either
InstallLocationordirname(DisplayIcon)), while on macOS it returns the.appbundle path (e.g./Applications/Foo.app).The difference is subtle but important for callers that might want to append
cli.bat/Contents/MacOS/...:
- Today,
devtools.tsonly uses this helper on Windows, so it’s fine.- Future callers might assume “installation directory” means the same thing on all platforms.
Consider either unifying the semantics (always return the directory that directly contains the main executable) or explicitly documenting the current platform‑specific behavior in the JSDoc so it’s clear how to consume the returned path.
124-180: Tighten quoting/escaping infindInUninstallRegistryPowerShell script
appNameis interpolated directly into the PowerShell snippet as:$target = "*${appName}*" ... Where-Object { $_.DisplayName -like $target }Even though current callers use a trusted constant, this will misbehave (or become an injection vector) if
appNameever contains quotes, backticks, or wildcard characters.To future‑proof this helper:
- Escape
appNamebefore embedding it in the script (e.g., replace"with\"and handle backticks), or- Pass it as an argument and build
$targetinside PowerShell without string interpolation, e.g. using single quotes and-like "*$($escapedName)*".Also, consider logging at debug level when the PowerShell call fails instead of completely swallowing errors; that will make diagnosing environment issues much easier.
Please verify whether
appNamemight ever come from configuration or user input; if so, harden the quoting/escaping as suggested.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/logics/devtools.ts(2 hunks)src/utils/index.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/logics/devtools.ts (1)
src/utils/index.ts (1)
findSoftwareInstallLocation(53-116)
这个不是问题，“*.app” 其实是一个文件夹而不是可执行文件本身，所以 *.app 视为 InstallLocation 应该也是合理的，符合这个函数的语义。 |
|
Tip For best results, initiate chat on the files or code changes. @Vanisper, thank you for the clarification! You're absolutely right — on macOS, ✏️ Learnings added
|
非常感谢🙏，我最近有点忙，可能要之后才能合并。 |
|
好的哈 |
2 participants
Description 描述
实现了一个工具函数 -
findSoftwareInstallLocation，用于通过可执行程序名称查询软件安装目录：Uninstall注册表（支持 32位 和 64 位），最后输出安装路径；最后真正的执行程序是上面得到的安装路径拼接上
cli.bat字符。Linked Issues 关联的 Issues
nothing
Additional context 额外上下文
nothing
Summary by CodeRabbit
New Features
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.