Fix handling of non-text editors

[tobous]

[FIX][I] #711 Add null checks for logic opening editors

Adjusts the logic opening editors and using the obtained editor objects
to correctly handle null return values.

Adds missing Nullable annotations to the corresponding methods.

Adjusts the corresponding methods javadocs to better reflect the
possible return values in different cases.

Partial fix for #711.

[FIX][I] Only save modified documents

Adjusts the logic to only save documents whose content has been
modified.

Adds the logic to check whether a document or virtual file is seen as
modified to the DocumentAPI.

This was introduced to avoid save operations on resources that can not
be represented by a document (e.g. binary files). Such cases are now
filtered out as such files will always be seen as unmodified by the API,
creating a better handling of such cases.

[FIX][I] Only set up user editor state for moved text files

Adjusts the logic handling the user editor state of moved resources to
only act if the resource is currently open in a text editor.

This change ensures that only the state of text editors is shared with
other Saros instances.

[FIX][I] #711 Ignore non-text editors for editor state

Adjusts the logic sharing the local editor state to ignore the opening
of non-text editors.

Adjusts the logic sharing the local editor state to ignore the closing
of non-text editors.

Moves the closing logic to the before listener to allow us to access the
editor before it is actually closed. This is needed to check the editor
type.

With this change, non-text editors are completely ignored by Saros/I.
This was done to match the Eclipse handling of such non-text editors.

Partial fix for #711.

[tobous]

The user editor state handling is still not consistent. I still have to adjust the handling of file creation, deletion, and move activities.

[tobous]

Updated PR to fix handling of closing non-text editors and moved non-text resources.

[srossbach]

Wouldn't it be better to check if the document is null here and then start to retrieve it ? Currently the save logic is present twice.

[tobous]

That is how it was done with the old implementation. I thought that this way was easier to read this way as there are two distinct logical paths through the code. The first part from l.195 to l.202 deals with the case that the document is known to the editor pool and the rest deals with the case that the document has to be requested.

But I can change it again if you prefer. That would then lead to the "has unsaved changes" check being run twice if the document can not be obtained from the editor pool.

This is due to the check in l.212 of whether the file has unsaved changes being used to filter out resources that can't be represented by a document and for which the getDocument(...) call in l.216 would therefore always return null.

Another approach would be to accept such cases and reduce the log level to debug in l.219.

[srossbach]

Have not read the API but what is the context when newEditor is null ?

[tobous]

Then no editor is selected (i.e. no editor is currently open). This information should also be transferred to other participants. This could lead to a situation where an unnecessary "no editors open" activity is send to the other participants if only non-text editors are open and the last of such editors is closed. But that should not be an issue.

[saros-infrastructure]

Here is an overview of what got changed by this pull request:

Complexity increasing per file
==============================
- intellij/src/saros/intellij/editor/LocalEditorManipulator.java  1
- intellij/src/saros/intellij/editor/EditorManager.java  1
- intellij/src/saros/intellij/eventhandler/editor/editorstate/ViewportAdjustmentExecutor.java  1
- intellij/src/saros/intellij/editor/LocalEditorHandler.java  2
- intellij/src/saros/intellij/eventhandler/editor/editorstate/EditorStatusChangeActivityDispatcher.java  2
         

See the complete overview on Codacy

[tobous]

@srossbach Do you still have any issues with this PR?