-
Notifications
You must be signed in to change notification settings - Fork 399
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add completely delete file option #4326
base: main
Are you sure you want to change the base?
Conversation
|
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint
warning [email protected]: This version is no longer supported. Please see https://eslint.org/version-support for other options. 概述演练这个拉取请求主要修改了文件删除的确认机制。移除了 变更
可能相关的 PR
建议标签
建议审阅者
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (5)
packages/workspace-edit/src/browser/workspace-edit.service.ts (1)
301-304
: 建议优化条件逻辑的可读性!当前的条件逻辑较为复杂,建议重构以提高可读性。
建议修改为:
- useTrash: - options.completelyDelete !== undefined - ? !options.completelyDelete // 如果 completelyDelete 被定义,使用其相反值 - : !(isWindows && this.oldResource.path.name === 'node_modules'), // 否则使用原有逻辑, + useTrash: (() => { + // 如果指定了完全删除选项,则不使用回收站 + if (options.completelyDelete !== undefined) { + return !options.completelyDelete; + } + // Windows 上的 node_modules 文件夹不使用回收站 + if (isWindows && this.oldResource.path.name === 'node_modules') { + return false; + } + // 默认使用回收站 + return true; + })(),packages/i18n/src/common/zh-CN.lang.ts (1)
58-60
: 确认新增的本地化文案符合预期!新增的完全删除相关的本地化文案清晰明确,并且与现有的移入回收站删除提示保持一致的风格。建议可以考虑在"彻底删除"的文案中添加警示性的提示,比如"此操作无法撤销"。
packages/file-tree-next/src/browser/services/file-tree-model.service.ts (3)
980-997
: 完全删除文件的确认对话框实现确认对话框的实现逻辑清晰,但建议进行以下改进:
- 建议将确认对话框的消息文本抽取为常量,以便于维护和复用。
- 建议添加文件数量的显示,当文件数量超过5个时,显示总数。
+const MAX_DISPLAY_FILES = 5; +const getDeleteFilesMessage = (uris: URI[], isCompletelyDelete: boolean) => { + const fileCount = uris.length; + const displayFiles = uris.slice(0, MAX_DISPLAY_FILES) + .map((uri) => uri.displayName) + .join(','); + const suffix = fileCount > MAX_DISPLAY_FILES ? + ` ... (共${fileCount}个文件)` : ''; + return `[ ${displayFiles}${suffix} ]`; +}; - const deleteFilesMessage = `[ ${uris - .slice(0, 5) - .map((uri) => uri.displayName) - .join(',')}${uris.length > 5 ? ' ...' : ''} ]`; + const deleteFilesMessage = getDeleteFilesMessage(uris, true);
1045-1045
: 确保完全删除参数正确传递删除操作的参数传递正确,但建议添加类型注释以提高代码可读性。
- this.deleteFile(root.node, root.path, !!this.corePreferences['explorer.confirmCompletelyDelete']).then((v) => { + this.deleteFile( + root.node, + root.path, + !!this.corePreferences['explorer.confirmCompletelyDelete'] as boolean + ).then((v) => {
1059-1062
: 完全删除功能的方法实现方法实现逻辑正确,但建议添加错误处理和日志记录。
async deleteFile(node: File | Directory, path: URI | string, completelyDelete?: boolean): Promise<boolean> { const uri = typeof path === 'string' ? new URI(path) : (path as URI); + try { const error = await this.fileTreeAPI.delete(uri, completelyDelete); if (error) { this.messageService.error(error); return false; } + } catch (e) { + this.messageService.error(`删除文件失败: ${e.message}`); + return false; + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
packages/core-browser/src/core-preferences.ts
(2 hunks)packages/file-tree-next/src/browser/services/file-tree-api.service.ts
(1 hunks)packages/file-tree-next/src/browser/services/file-tree-model.service.ts
(3 hunks)packages/file-tree-next/src/common/index.ts
(1 hunks)packages/i18n/src/common/en-US.lang.ts
(1 hunks)packages/i18n/src/common/zh-CN.lang.ts
(1 hunks)packages/workspace-edit/src/browser/workspace-edit.service.ts
(2 hunks)packages/workspace-edit/src/common/index.ts
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/core-browser/src/core-preferences.ts
🧰 Additional context used
🪛 Biome (1.9.4)
packages/file-tree-next/src/common/index.ts
[error] 25-25: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
🔇 Additional comments (5)
packages/file-tree-next/src/common/index.ts (1)
25-25
: 接口更改符合预期!
delete
方法签名的更改恰当地添加了可选的completelyDelete
参数,保持了向后兼容性。🧰 Tools
🪛 Biome (1.9.4)
[error] 25-25: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
packages/workspace-edit/src/common/index.ts (1)
30-30
: 选项定义清晰明确!在
IResourceFileEdit
接口中添加completelyDelete
选项,类型定义准确,保持了与其他选项的一致性。packages/file-tree-next/src/browser/services/file-tree-api.service.ts (1)
204-212
: 实现简洁有效!
delete
方法的实现正确地处理了completelyDelete
参数,并保持了良好的错误处理机制。packages/workspace-edit/src/browser/workspace-edit.service.ts (1)
200-200
: 类型定义符合接口规范!在
ResourceFileEdit
类中添加completelyDelete
选项,与接口定义保持一致。packages/i18n/src/common/en-US.lang.ts (1)
57-59
: 英文本地化文案翻译准确!英文版本的完全删除相关文案与中文版本含义对应,用词专业规范。
看起来你期望的“彻底删除”这个功能是默认实现了的,只是在文案上看起来有歧义。 |
Types
Background or solution
Since we can't recover files from nas drives, we added the option to delete files completely.
Changelog
Summary by CodeRabbit
新功能
本地化
改进