search

brackets-archive / bracketsIssues

Archive of issues in brackets.
0 stars 0 forks source link

[CLOSED] Prevent duplicate tree nodes when renaming the only file in a directory #5928

Open core-ai-bot opened 3 years ago

core-ai-bot commented 3 years ago

Issue by jasonsanjose Wednesday Jan 15, 2014 at 00:32 GMT Originally opened as https://github.com/adobe/brackets/pull/6522

Use jstree open_node in corner case #6474 where renaming the only file in a directory caused duplicate tree nodes to appear

jasonsanjose included the following code: https://github.com/adobe/brackets/pull/6522/commits

core-ai-bot commented 3 years ago

Comment by jasonsanjose Wednesday Jan 15, 2014 at 00:35 GMT

Assigning@couzteau for review

core-ai-bot commented 3 years ago

Comment by couzteau Wednesday Jan 15, 2014 at 02:16 GMT

will look at this later tonight.

core-ai-bot commented 3 years ago

Comment by couzteau Wednesday Jan 15, 2014 at 06:15 GMT

Done with initial review. The fix works great. I have just one Q regarding the additional redraw, see comment above. While testing I have seen a few cases where the tree wouldn't update at all after renaming a file. In these situations I was also not hitting any breakpoints in the _fileSystemChange handler. Couldn't really repro, but I have seen it a couple of times. That would be a different issue I think, but for the time being I cannot really put my finder on it.

core-ai-bot commented 3 years ago

Comment by jasonsanjose Wednesday Jan 15, 2014 at 06:44 GMT

The additional redraw (to fix the selection marker drawing) is necessary since the first synchronous redraw will happen when the nodes are deleted for this event. This additional async redraw happens after jstree calls into the FileSystem when completing open_node.

core-ai-bot commented 3 years ago

Comment by jasonsanjose Wednesday Jan 15, 2014 at 06:46 GMT

As far as not hitting breakpoints in _fileSystemChange, that would be a different issue. Feel free to merge if I answered your questions.

core-ai-bot commented 3 years ago

Comment by couzteau Wednesday Jan 15, 2014 at 20:23 GMT

Thanks, I understand now. Actually it dawned on me lats night that the redraw handler and the open node call are directly related ;) - Of course they are! Thanks. This is looking good. The fix is working well and the tests are passing. Merging.