Split View (Same Document)

[swmitra]

This feature enables users of Brackets to open same document in both the editor panes and also enables extension developers to create full editors for documents under different containers to make full use of the code hinting features already available for Brackets.

For more details about the feature , please refer to https://trello.com/c/GezHZcCx/390-m-split-view-same-document#

Feature Details

• Same document will be used by all the full editors to seamlessly synchronize change sets.
• Undo/Redo stack also gets synchronized ( although not same ).
• When opened in both the panes , the same document gets listed in both the working sets.
• The focused full editor acts as master editor for the document at any point of time.

---

Implementation

The core idea of this implementation is to handle 'masterEditor' tied to a document instance. 'Document' in the new implementation maintains a list of full editors and exposes private API to switch between full editors as master editor. Full editors who are tied to the document , marks itself as master editor when focused.

---

Pending Tasks

• Fix existing unit tests.
• Add new unit test for creation.

[nethip]

@swmitra Thanks for this PR. Very nice addition to Brackets!

@marcelgerber @zaggino @sprintr @petetnt @peterflynn Could you guys take some time in reviewing this?

[nethip]

Tagging @abose

[nethip]

@swmitra Please fix the JSHint errors.

[petetnt]

Began reviewing this against the current master 40ad8e9

Most recently used (MRU)-list is partially broken

The _makeFileMostRecent-method in in view/MainViewManager.js#L328 is logging tons of C:/foo/bar.js duplicated in mru list-notifications, because _findFileInMRUList-method only checks for the file.fullPath, instead of that combined with the pane ID, same way as the index check does in view/MainViewManager.js#L337.

Same goes for example:

• the comparison in view/MainViewManager.js#L855,
• findInViewListMRUOrder in view/Pane.js#L711
• most likely all the other MRU related methods that just check the filePath without the paneID.

Until this is fixed, for example navigating the opened files with CTRL+Tab has some really unexpected behaviors.

Has some issues with my implementation of #11749

As this feature and my feature were done at the same time, these kinds of issues were to be expected 👯

Outside of the MRU-issues, this feature seems to work (from a 30 minute test session) very well. There are some issues in the flip-view scenario though:

1. Open foo.js in pane1 //works
2. Open foo.js in pane2 //works
3. Flip foo.js#pane2 to pane1 //works
4. Flip foo.js#pane1 to pane2

This occurs:

For some reason the file did get flipped, but the other pane ended up in this weird limbo state. The situation can be eventually recovered to normal.

The issue occurs in view/Pane.js#L249 so the culprit is those methods should be checked out (MainVIewManager._moveview, CommandManager.execute(Command.FILE_OPEN...) and anything that's listening to viewListChange-events.).

As I am the author of the other feature I am of course happy to help with that as soon as I have some more time 👍

PerfUtils issues?

 Recursive tests with the same id are not supported. Timer id: [reent 1] CodeInspection:    C:/Users/Pete/Desktop/foo/foo.js
PerfUtils.js:144 Recursive tests with the same id are not supported. Timer id: [reent 1] CodeInspection 'ESLint':   C:/Users/Pete/Desktop/foo/foo.js
MainViewManager.js:347 C:/Users/Pete/Desktop/foo/bundle.js duplicated in mru list
MainViewManager.js:347 C:/Users/Pete/Desktop/foo/bundle.js duplicated in mru list

JSHint issues

1. Create foo.js
2. Open foo.js in pane1 and 2
3. write something ->

Exception in 'editorChange' listener on Editor TypeError: Cannot read property 'length' of undefined TypeError: Cannot read property 'length' of undefined
    at Object.maybeIdentifier (file:///C:/Program%20Files%20(x86)/Brackets/dev/src/extensions/default/JavaScriptCodeHints/HintUtils.js:67:28)
    at Session.getQuery (file:///C:/Program%20Files%20(x86)/Brackets/dev/src/extensions/default/JavaScriptCodeHints/Session.js:257:31)
    at JSHints.getHints (file:///C:/Program%20Files%20(x86)/Brackets/dev/src/extensions/default/JavaScriptCodeHints/main.js:477:35)
    at _updateHintList (file:///C:/Program%20Files%20(x86)/Brackets/dev/src/editor/CodeHintManager.js:426:40)
    at _handleChange (file:///C:/Program%20Files%20(x86)/Brackets/dev/src/editor/CodeHintManager.js:618:17)
    at Editor.trigger (file:///C:/Program%20Files%20(x86)/Brackets/dev/src/utils/EventDispatcher.js:222:40)
    at Editor._handleEditorChange (file:///C:/Program%20Files%20(x86)/Brackets/dev/src/editor/Editor.js:870:14)
    at file:///C:/Program%20Files%20(x86)/Brackets/dev/src/editor/Editor.js:412:18
    at Editor.trigger (file:///C:/Program%20Files%20(x86)/Brackets/dev/src/utils/EventDispatcher.js:222:40)
    at file:///C:/Program%20Files%20(x86)/Brackets/dev/src/editor/Editor.js:941:18
    at CodeMirror.signal (file:///C:/Program%20Files%20(x86)/Brackets/dev/src/thirdparty/CodeMirror/lib/codemirror.js:8112:49)
    at endOperation_finish (file:///C:/Program%20Files%20(x86)/Brackets/dev/src/thirdparty/CodeMirror/lib/codemirror.js:3127:7)
    at endOperations (file:///C:/Program%20Files%20(x86)/Brackets/dev/src/thirdparty/CodeMirror/lib/codemirror.js:3021:7)
    at endOperation (file:///C:/Program%20Files%20(x86)/Brackets/dev/src/thirdparty/CodeMirror/lib/codemirror.js:3004:7)
    at runInOp (file:///C:/Program%20Files%20(x86)/Brackets/dev/src/thirdparty/CodeMirror/lib/codemirror.js:3137:15)
    at TextareaInput.copyObj.poll (file:///C:/Program%20Files%20(x86)/Brackets/dev/src/thirdparty/CodeMirror/lib/codemirror.js:1434:7)

[busykai]

You need to use Brackets with JSLint enabled when contributing to Brackets. Please re-check your code. In this specific case, there's a space missing between if and ( which JSLint would complain about.

[zaggino]

Recursive tests with the same id are not supported. is not an extension issue, it's a core issue. I'm guessing in this case it's the fact that eslint is called for twice (for each view) and PerfUtils id is created from the file name, so id is the same in both cases.

[busykai]

@zaggino, I though I fixed that in #9392. I'll take a look at what this does to cause it again.

[petetnt]

@zaggino yeah, I explained that pretty badly as I was in a bit of a hurry and might have jumped into conclusions as linting the same file without any extensions enabled (with JSLint) seems to work fine, but doing the same while using brackets-eslint seemed to end up in EventDispatcher.js:224 Exception.

But you are correct, it doesn't have anything to do with ESLint itself, my bad. I've updated the post.

[petetnt]

More issues:

De-synchronized files

1. Open bar.js in pane 1 and 2
2. Type `var helloworld = "Hello, world!" to pane 1 (should duplicate to pane 2)
3. Open foo.js in pane 2
4. Modify the pane 1 text to var foobar = "Foo, bar!
5. Open bar.js in pane 2 again
6. Start typing in pane 1
7. Destruction ensues as letters get duplicated in pane 1 and the cursor position is completely off in pane 2.

Working set behavior

Working set should be synchronized too: if a file gets modified (is marked dirty) in pane 2 and is open in pane 1, it should appear in both working sets.

After closing a file, the pane states that it is still the same file but shows the contents of the other file

Haven't been able to repeat this in a repeatable manner, but sometimes when closing files that are duplicated, pane text and the contents of the file don't match. Might be related to the working set behavior. Saving the file in this state brings tons of unsuspected behaviors.

[swmitra]

Thanks a lot guys for the quick start and please excuse me for responding so late.
@petetnt I will fix the mru list issue and also go through the flip scenarios once I get back to work.
@busykai I will fix all those lint issues as soon as I come back.

[brenovieira]

+1

I hope you merge this soon =]

[swmitra]

@brenovieira Sure! Sorting out few issues with the changes.

[swmitra]

@zaggino @petetnt Updated the PR with MRU List fix.
@petetnt Can you please let check it once more with your feature?
@busykai Fixed all the lint warnings.
I have done some changes to DocumentCommandHandler to ensure it doesn't display the File Save dialog if file close is requested for a pane and the file is listed in other panes as well.
I thought that would be a logical behavior.

[petetnt]

@swmitra seems to be working with my feature now 👍

I did run into that de-syncronized issue described here: #11820 (comment) again

Also found a new bug:

1. Open foo.js to pane 1 and 2
2. Type something in pane 1 -> gets added to left working set
3. Switch to pane 2 and press CTRL+Z / undo -> gets added to right working set

[swmitra]

@petetnt To me it seems like expected as undo/redo stack should be in sync. If I am missing something here please let me know.

I have tried to reproduce de-synchronized issue but it's intermittent. Looks like change listner is getting registered multiple times. I am looking into this and push the required changes soon.

[petetnt]

@swmitra Check out this screenshot flow if it's any help (redo and undo stack work as expected, the error is in the working set):

Write in the first panel

Write in the second panel

Press CTRL+Z / Undo when second panel is still active (you might need to do it a few times, it doesn't always trigger for me):

Not sure if it would make sense that the working sets would be in sync with each other altogether: if foo.js is modified in left panel while it's also in right panel, both working sets should show the file. Then again it might just create additional noise to the working sets.

[ryanstewart]

Just took a look at this, sorry for the delay. I didn't do a ton of testing, but so far it looks great and is a very nice addition.

Open question on the shortcuts: Should we use Left Pane/Right Pane instead of First Pane/Second Pane? I suppose the latter scales better for if/when we get multiple splits.

[swmitra]

@petetnt Finally I have modified the working set behavior in two use cases.

• If same document is opened in both the panes as view only and not edited yet, first edit in any of the panes will force the document to get added to the other panes working set along with the originating panes working set. ( you have already asked for this :-) )
• If a document is opened in one pane and present in that working set with dirty flag set to true, while opening the same document in other pane, it gets added to the other panes working set as well.

[abose]

There is one pending issue, if the same doc is open in both panes, after doing some edits, if you do undo till the start state, the document will not get undirtied one all changes are undone.

[swmitra]

There are two pending issues @abose @nethip @petetnt -

• In some open/close combination ( not yet zeroed on the actual sequence ) of the same document in multiple panes and inline widgets, change list gets applied to the editors multiple times.
• if the same doc is open in both panes, after doing some edits, if you do undo till the start state, the document will not revert back to clean state when all changes are undone.

I am working on both of these and hopefully today we can resolve both these issues and probably good to go for a merge.

[swmitra]

@petetnt Silly me :-( . Fixed now by handling custom views. Need to do some more unit testing on this area though.

[petetnt]

@swmitra thanks for the quick fix!

[petetnt]

@swmitra Just now, after getting the Possible memory leak warnings for a while, another file foo.js showed the content of another file bar.js (that was open on Pane 2 at the same time). foo.js didn't start to show the correct context until Brackets was restarted (the situation persisted despite closing all the related files in the working set).

[swmitra]

@petetnt Fixed the possible memory leak issue. Found two new problems(?) while testing with the flip feature.

• If I open a document as view only (without adding it to working set ) and then do a flip , it gets added to the other panes working set. I thought it should be again view only as it was not in working set.
• If I do any edit in a document and then try to close it using the close link in flip toolbar, it doesn't ask for save confirmation and marks the document clean. So the edited content is lost. I was trying to debug this issue and noticed that handleFileClose handler in DocumentCommandHandlers is never getting called in this case. But if I try to close the doc in same scenario using close link in working set it does go though the FILE_CLOSE command which takes care of these things.

This might be an issue with the integration of both these features , but please have a look to identify any obvious pointer which I would have missed during debugging.

[swmitra]

@petetnt I have fixed the second problem mentioned in my earlier post. It was happening because paneId was not being sent as part of command data while invoking FILE_CLOSE. Included the pane info now.

[petetnt]

@swmitra I think the first issue should be fixed by checking if the current file is in the working set before triggering viewListChanges here: https://github.com/adobe/brackets/blob/master/src/view/Pane.js#L252

I think that might be an error in the original implementation too, not directly related to integration itself

[nethip]

LGTM. Thanks @swmitra

This is a great addition to Brackets! Thanks @petetnt @busykai @zaggino for helping us out in the code review as well as testing activities. We really appreciate it!

[depiction]

How is this feature used? I can't find any documentation and am not sure how to view a file in both panes in 1.6.

[petetnt]

@depiction Open Split View and open a file to the first pane. Then focus the other pane and open the same file again (from the file tree). You should now see the same file on both panes.

[depiction]

@petetnt Thanks.

[DariusNorv]

Hi, how can I disable this feature? Unfortunately it's very unusual for me

[swmitra]

It's an explicit feature. You can always choose not to have the same doc on both the panes.

[petetnt]

To be honest it confused me too (at first) a lot before getting used to it (and now I quite enjoy it 😸)

Not sure how hard it would be to add a preference to it though. At best it would be just checking which pane to open on (if the file is already open).

[DariusNorv]

@swmitra When I choose a document to edit from working directory (double-click) it automatically opens in active pane. I prefer I my workspase to keep different file types in different panes, so it will be great to enable and disable this feature when needed

[Siphon880gh]

Works terrifically!

A tip for devs with many files in the File Tree and find it frustrating to find where the file is to open in the other panel:

First make sure the panel you want to open the file in as well - that it's active or in focus. Then right click the file at the top left panel (rather than the File Tree), then go to "Show in Finder" (on Mac or whatever equivalent on Windows). Open the file from there. It should open in the active panel! Now you have two panels editing the same file.

[paulmz]

I saw a single post above asking this question but no real answer. Is there any way to disable this "feature"? I personally do not like it as it constantly takes files that are already open in one pane and then opens them in the other pane when clicking tabs. This is insanely annoying and disrupts my workflow.