Add indication for externally deleted files #1999
Conversation
tom-ludwig
requested review from
austincondiff,
thecoolwinter and
tom-ludwig
| @@ -57,7 +60,7 @@ struct EditorTabView: View { | |||
| /// The item associated with the current tab. | |||
| /// | |||
| /// You can get tab-related information from here, like `label`, `icon`, etc. | |||
| private var item: CEWorkspaceFile | |||
| private let item: CEWorkspaceFile | |||
There was a problem hiding this comment.
Might be an idea to rename this to something more descriptive?
There was a problem hiding this comment.
To be honest my eye just got caught by the var keyword, so I decided to change it to let for the sake of its usage. But I'll push update on the name also – "item" is a no-go naming for sure.
| /// | ||
| /// Often returns ``FSEvent/changeInDirectory`` as `FSEventStream` returns | ||
| /// Often returns ``[FSEvent/changeInDirectory]`` as `FSEventStream` returns |
There was a problem hiding this comment.
I'm not sure I understand this sentence 😅
There was a problem hiding this comment.
I think you might be loosing the other part of it on the next line 🙂
/// Often returns ``[FSEvent/changeInDirectory]`` as `FSEventStream` returns
/// `kFSEventStreamEventFlagNone (0x00000000)` frequently without more information.This docstring was set before my changes, I only changed the type of returning value
| func fileManagerUpdated(updatedItems: Set<CEWorkspaceFile>) { | ||
| guard let parent = item.parent else { | ||
| return | ||
| } | ||
|
|
||
| if updatedItems.contains(parent) { | ||
| isDeleted = item.doesExist == false | ||
| } | ||
| } |
There was a problem hiding this comment.
I believe this can be simplified, and I’m fairly certain it should be updated on the main thread as well
| func fileManagerUpdated(updatedItems: Set<CEWorkspaceFile>) { | |
| guard let parent = item.parent else { | |
| return | |
| } | |
| if updatedItems.contains(parent) { | |
| isDeleted = item.doesExist == false | |
| } | |
| } | |
| func fileManagerUpdated(updatedItems: Set<CEWorkspaceFile>) { | |
| if let parent = item.parent, updatedItems.contains(parent) { | |
| isDeleted = item.doesExist == false | |
| } | |
| } |
There was a problem hiding this comment.
I think that depends on whether you are a fan of explicit guard's everywhere (like me) or not. But I think you are right and semantics of your suggestion is more clean about the logic and intention of this block.
And of course main thread is a must – missing it was just my mistake.
pro100filipp
dismissed stale reviews from matthijseikelenboom and austincondiff
via
b3d8a48
|
Implemented all suggested changes |
Description
Related Issues
Checklist
Screenshots
1936.mov