Added projectRefresh event when project tree is refreshed but project root has not changed. #4815
Conversation
… root has not changed.
ghost
assigned 
|
If the idea is to get notifications when files are added/deleted/renamed, hanging those notifications off of ProjectManager doesn't seem ideal. But OTOH, once we add file watchers we'll probably have a whole different API for file-change notifications, so maybe it's not so bad to notify from ProjectManager for now & then deprecate it later...? |
|
@peterflynn @zaggino i already added an extension which updates the file tree, maybe when the new proposed extension api is in place you could hook in to change events there. |
|
@peterflynn @WebsiteDeveloper my problem with hooking on any file change/add/delete/rename actions is that when user switches branches on git, none of this occurs. All files might be exactly the same on two different branches. Right now user has to change branches outside of brackets (I haven't implemented that yet but it's in plan though) and when he goes back to brackets, right clicking on the file tree and using "Refresh File Tree" won't help him, because there's no event to hook on. Maybe the event should be placed somewhere else but this seemed like a good place to me and it works fine. |
| @@ -920,6 +920,7 @@ define(function (require, exports, module) { | |||
| $(exports).triggerHandler({ type: "projectOpen", promises: promises }, [_projectRoot]); | |||
| $.when.apply($, promises).then(result.resolve, result.reject); | |||
| } else { | |||
| $(exports).triggerHandler("projectRefresh", _projectRoot); | |||
There was a problem hiding this comment.
I wonder if it would make sense to trigger this in the projectOpen case as well (i.e., move it out of the if), so someone who just wants to know when the tree has been updated wouldn't have to listen to both projectOpen and projectRefresh.
There was a problem hiding this comment.
Also, could you add documentation for this to the comment at the top of the file where we list other events dispatched by ProjectManager?
There was a problem hiding this comment.
I have multiple actions assigned to projectOpen and they are not all the same as projectRefresh so putting it out of the if would make some things run when they are actually not required. I'd prefer to keep these events separate.
There was a problem hiding this comment.
On the other side, projectRefresh is (in my case) a subset of projectOpen so putting it out of the if would be fine, in a case that projectRefresh handlers will be always launched after projectOpen handlers.
There was a problem hiding this comment.
Yeah, I guess I can see it either way. Logically, "refresh" is different from "open", so it makes sense to keep them separate.
|
Initial review done. I think it's okay to add this because it's different from individual file notifications--it's specifically saying that the UI of the file tree has been torn down and rebuilt, and extensions that are annotating the tree might need to reapply themselves. |
|
@njx Added the docs for the event. |
|
@njx can you take a second look at this? I can move the event out of the |
|
Sorry, been busy. Will try to get to this soon. |
|
Seems fine. I'm going to push up another commit to tweak the comment after merging this. Thanks. |
Added projectRefresh event when project tree is refreshed but project root has not changed.
|
Thanks @njx |
I need an event like this for my git extension which hooks on project tree and displays branch information from git. I have not found any other way to hook onto this.