Renaming twice a file in the working set shows wrong name

[jbalsas]

If you rename a file that is on the working set twice, Brackets seems to be updating the name twice

This may be related to #1870

Steps to reproduce

1. Create a new file foo.bot
2. Open the file so that it appears on the working set
3. Rename the file to foo.b
4. Rename the file to foo.bar

Results:

• The file in the working set shows the name foo.barar, even if we close and open again the file.
• Reloading Brackets with the file in the working set shows the "Error opening file" window saying that foo.barar could not be found.
• As a weirdness plus, switching to other window and then back to Brackets removes foo.barar from the working set :)

Expected:
The file in the working set should show foo.bar

[jbalsas]

Hi,

I've pushed a possible solution, but I've found some other issues that I'd like to discuss.

This error happens when the file of a document gets updated twice inside DocumentManager.notifyPathNameChanged: once while looping over _openDocuments and once again while looping over _workingSet.

However, this only happens with documents opened in the current session for which the file entry in _openDocuments and in _workingSet seem to be the same. For documents that were in the working set initially, both file entries seem to be different ones and going over them twice is no problem.

It may be worth looking into it deeper to consolidate both cases.

[pthiess]

Assigned to @gruehle

[jbalsas]

@gruehle Thanks for your comments in #1975. Now that indexOf in FileUtils makes total sense...

I've been looking more closely at this. This is what I've found:

• When a project is opened, the opened files are added to _workingSet through the call to addListToWorkingSet.
• Later on, when getDocumentForPath is called, a new fileEntry is created and added to the Document. When the promise is solved, setCurrentDocument and then addToWorkingSet are executed.
• As the first check if (findInWorkingSet(file.fullPath) !== -1), in addToWorkingSet is true, the new fileEntry just gets tossed and we end up with two different fileEntries in _workingSet and in _openDocuments.
• Documents that were not loaded through the preferences, add the reference to the fileEntry in getDocumentForPath and later, after the promise gets resolved, it gets added to _workingSet in the addToWorkingSet method, resulting in having the same fileEntry in both places.

It seems like we should have 3 ways to solve it, then:

1. Check which paths got already renamed in notifyPathNameChanged so we don't rename them twice as you suggested.
2. Make sure fileEntry in _openDocuments and _workingSet are different. This can be achieved by changing line 210 to _workingSet.push(new NativeFileSystem.FileEntry(file.fullPath)); in DocumentManager. Are there any downsizes for this?
3. Make sure the fileEntry is the same. This can be achieved by replacing the fileEntry in addToWorkingSet if it already exists (instead of just discarding it), and calling the method from getDocumentForPath if the document is in the workingSet. Additionally, we'd remove the loop over the working set documents in notifyPathNameChanged.

I've already checked and all 3 solutions seem valid.

What do you think? Which option feels better for a fix? To me, 2 and 3 are more consistent as we consolidate the behavior for documents in the workingSet one way or another, but maybe I'm still missing something here...

[gruehle]

@jbalsas I agree that options 2 and 3 are both better than option 1.

Eventually this will all change when we get file watchers and have proper model objects that represent a file. In the meantime, it seems like option 2 is the simplest to implement and enforce.

Thanks for investigating!

[peterflynn]

Definitely some interesting philosophical questions here, for down the road... Glenn and I talked about this for a bit today. I've left a long comment in https://trello.com/card/8-file-directory-watching/4f90a6d98f77505d7940ce88/292 with notes.

[gruehle]

Confirmed fixed. Closing. Thanks @jbalsas for the fix!