fix(file): 缺少默认打开程序时使用记事本兜底#3087
Conversation
审阅者指南(在小型 PR 上折叠显示)审阅者指南在打开文件时增加错误处理逻辑:当没有关联的默认程序时回退到使用记事本(Notepad)打开,防止在打开某些文件时发生崩溃。 OpenPath 回退到 Notepad 的时序图sequenceDiagram
participant Caller
participant Basics
participant Process
Caller->>Basics: OpenPath(path, workingDirectory)
Basics->>Process: Start(ProcessStartInfo pathPsi)
alt associated_program_exists
Process-->>Basics: process_started
else no_associated_program_and_file_exists
Basics->>Process: Start(ProcessStartInfo notepadPsi)
end
文件级变更
与关联 Issue 的匹配情况评估
可能相关的 Issue
提示与命令与 Sourcery 交互
自定义你的体验访问你的 控制面板 以:
获取帮助Original review guide in EnglishReviewer's guide (collapsed on small PRs)Reviewer's GuideAdds error-handling around file opening to fall back to Notepad when there is no associated default program, preventing crashes when opening certain files. Sequence diagram for OpenPath fallback to NotepadsequenceDiagram
participant Caller
participant Basics
participant Process
Caller->>Basics: OpenPath(path, workingDirectory)
Basics->>Process: Start(ProcessStartInfo pathPsi)
alt associated_program_exists
Process-->>Basics: process_started
else no_associated_program_and_file_exists
Basics->>Process: Start(ProcessStartInfo notepadPsi)
end
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - 我发现了 1 个问题,并给出了一些高层面的反馈:
- 当前的回退路径假定
path一定是文件;建议在执行notepad.exe回退前增加一个!Directory.Exists(path)判断,这样目录路径(或不存在的路径)就不会触发不合适的记事本启动。 1155这个原生错误码是一个魔法数字;建议用一个具名常量或辅助方法来替代(例如NoApplicationAssociatedErrorCode),以便更清晰地表达意图,并避免在代码库中到处散落原始的 Win32 错误码。
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- 当前的回退路径假定 `path` 一定是文件;建议在执行 `notepad.exe` 回退前增加一个 `!Directory.Exists(path)` 判断,这样目录路径(或不存在的路径)就不会触发不合适的记事本启动。
- `1155` 这个原生错误码是一个魔法数字;建议用一个具名常量或辅助方法来替代(例如 `NoApplicationAssociatedErrorCode`),以便更清晰地表达意图,并避免在代码库中到处散落原始的 Win32 错误码。
## Individual Comments
### Comment 1
<location path="PCL.Core/App/Basics.cs" line_range="165" />
<code_context>
+ {
+ Process.Start(psi);
+ }
+ catch (Win32Exception ex) when (ex.NativeErrorCode == 1155 && File.Exists(path))
+ {
+ Process.Start(new ProcessStartInfo
</code_context>
<issue_to_address>
**suggestion:** 避免直接使用 Win32 错误码 1155 这种魔法数字。
直接使用原始的 `1155` 会让这个条件更难阅读和维护。请使用具名常量(例如 `ERROR_NO_ASSOCIATION = 1155`)或一个共享的枚举/集中定义的 Win32 错误码集合,以清晰表达意图并避免拼写错误。
建议实现方式:
```csharp
catch (Win32Exception ex) when (ex.NativeErrorCode == ErrorNoAssociation && File.Exists(path))
```
1. 在合适的共享位置为该 Win32 错误码定义一个具名常量。例如,在包含此方法的同一个类中:
- `private const int ErrorNoAssociation = 1155;`
如果已经有集中存放 Win32 错误码的位置,则将其添加到那里(例如 `Win32ErrorCodes` 静态类或枚举),并相应更新调用处:
- `ex.NativeErrorCode == Win32ErrorCodes.ErrorNoAssociation`。
2. 确保常量名称与条件中使用的名称一致(`ErrorNoAssociation` 或 `Win32ErrorCodes.ErrorNoAssociation`)。
</issue_to_address>帮我变得更有用!请在每条评论上点 👍 或 👎,我会根据你的反馈改进后续的评审。
Original comment in English
Hey - I've found 1 issue, and left some high level feedback:
- The fallback path currently assumes
pathis a file; consider guarding thenotepad.exefallback with a!Directory.Exists(path)check so directories (or non-existent paths) don’t trigger an inappropriate Notepad launch. - The
1155native error code is a magic number; consider replacing it with a named constant or helper (e.g.,NoApplicationAssociatedErrorCode) to make the intent clearer and avoid scattering raw Win32 codes through the codebase.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The fallback path currently assumes `path` is a file; consider guarding the `notepad.exe` fallback with a `!Directory.Exists(path)` check so directories (or non-existent paths) don’t trigger an inappropriate Notepad launch.
- The `1155` native error code is a magic number; consider replacing it with a named constant or helper (e.g., `NoApplicationAssociatedErrorCode`) to make the intent clearer and avoid scattering raw Win32 codes through the codebase.
## Individual Comments
### Comment 1
<location path="PCL.Core/App/Basics.cs" line_range="165" />
<code_context>
+ {
+ Process.Start(psi);
+ }
+ catch (Win32Exception ex) when (ex.NativeErrorCode == 1155 && File.Exists(path))
+ {
+ Process.Start(new ProcessStartInfo
</code_context>
<issue_to_address>
**suggestion:** Avoid using the magic Win32 error code 1155 directly.
Using the raw `1155` makes this condition harder to read and maintain. Please use a named constant (e.g. `ERROR_NO_ASSOCIATION = 1155`) or a shared enum/centralized definition for Win32 error codes to clearly document the intent and avoid typos.
Suggested implementation:
```csharp
catch (Win32Exception ex) when (ex.NativeErrorCode == ErrorNoAssociation && File.Exists(path))
```
1. Define a named constant for this Win32 error code in an appropriate shared location. For example, within the same class that contains this method:
- `private const int ErrorNoAssociation = 1155;`
or if there is an existing central place for Win32 error codes, add it there (e.g., a `Win32ErrorCodes` static class or enum) and update the usage accordingly:
- `ex.NativeErrorCode == Win32ErrorCodes.ErrorNoAssociation`.
2. Ensure the constant name matches what you use in the condition (`ErrorNoAssociation` or `Win32ErrorCodes.ErrorNoAssociation`).
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 16c8f222cd
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
This PR resolves #3086.
Summary by Sourcery
Bug Fixes:
Original summary in English
Summary by Sourcery
Bug Fixes: