The Inline Diff Sidebar should be made visible / not visible by calling the API method EditorPreferences.setDiffSidebar(mime, visible);
.
Currently this is done inside ADS by calling VersioningCache.setBase(JEditorPane, original, base = null)
, but in this case the Diff Side Bar visibility is not applied on already opened editor panes. Actually using this solution the Diff Sidebar is always visible (space is allocated for it) but with no visible diffs (coloured areas).
VersioningCache.setBase() is only called in CEditorPanelView. It is called in three places in fx() : reloadInlineDiff(), doSave(), doOpen(). I'm assuming in the case where base==null then instead of calling this method, we call EditorPreferences.setDiffSidebar().
I am not sure that is correct. Wouldn't that disable the inline diff for all open editors with that mime type?
VersioningCache.setBase() is only called in CEditorPanelView. It is called in three places in fx() : reloadInlineDiff(), doSave(), doOpen(). I'm assuming in the case where base==null then instead of calling this method, we call EditorPreferences.setDiffSidebar().
I am not sure that is correct. Wouldn't that disable the inline diff for all open editors with that mime type?
VersioningCache.setBase() is only called in CEditorPanelView. It is called in three places in fx() : reloadInlineDiff(), doSave(), doOpen().
I am not sure when do you call reloadInlineDiff() , but for doSave() and doOpen() is expected to call VersioningCache.setBase().
I'm assuming in the case where base==null then instead of calling this method, we call EditorPreferences.setDiffSidebar().
No, I didn't meant that. VersioningCache.setBase(JEditorPane, original, base = null);
still need to be called for unversioned files, no matter what value the "Inline Diff Sidebar" has for that mimetype.
I am not sure that is correct. Wouldn't that disable the inline diff for all open editors with that mime type?
That's exactly what toggling the Inline Diff Sidebar option should do, isn't it? That's why you need to call EditorPreferences.setDiffSidebar(mime, visible);
when user hits OK on the Options window, just as you do for other preferences.
The problem with current ADS v12 builds is that this API method is never called. Instead, it cuts the value of the base parameter and replace it with null if "Inline Diff Sidebar" is disabled for current mimetype. This is not the correct way of disabling the Inline Diff Sidebar for all editors having a specific mimetype and would work only on newly opened editors.
VersioningCache.setBase() is only called in CEditorPanelView. It is called in three places in fx() : reloadInlineDiff(), doSave(), doOpen().
I am not sure when do you call reloadInlineDiff() , but for doSave() and doOpen() is expected to call VersioningCache.setBase().
I'm assuming in the case where base==null then instead of calling this method, we call EditorPreferences.setDiffSidebar().
No, I didn't meant that. VersioningCache.setBase(JEditorPane, original, base = null);
still need to be called for unversioned files, no matter what value the "Inline Diff Sidebar" has for that mimetype.
I am not sure that is correct. Wouldn't that disable the inline diff for all open editors with that mime type?
That's exactly what toggling the Inline Diff Sidebar option should do, isn't it? That's why you need to call EditorPreferences.setDiffSidebar(mime, visible);
when user hits OK on the Options window, just as you do for other preferences.
The problem with current ADS v12 builds is that this API method is never called. Instead, it cuts the value of the base parameter and replace it with null if "Inline Diff Sidebar" is disabled for current mimetype. This is not the correct way of disabling the Inline Diff Sidebar for all editors having a specific mimetype and would work only on newly opened editors.
EditorPreferences.setDiffSidebar(mime, false) hides the UI component that displays the diff sidebar. So, regardless of the versioned state, the diff sidebar will be invisible.
VersioningCache just controls the versioning state so depending on the file contents, it might display something or not in the diff sidebar.
So, if versioning changes for a particular file and you do want to have the diff sidebar for a mimetype you just use VersioningCache (and you keep the diff sidebar visible).
If you just don't want the diff sidebar, use EditorPreferences.setDiffSidebar(mime, false).
Basically, in a MVC terminology EditorPreferences.setDiffSidebar controls the "view" while VersioningCache controls the "model".
EditorPreferences.setDiffSidebar(mime, false) hides the UI component that displays the diff sidebar. So, regardless of the versioned state, the diff sidebar will be invisible.
VersioningCache just controls the versioning state so depending on the file contents, it might display something or not in the diff sidebar.
So, if versioning changes for a particular file and you do want to have the diff sidebar for a mimetype you just use VersioningCache (and you keep the diff sidebar visible).
If you just don't want the diff sidebar, use EditorPreferences.setDiffSidebar(mime, false).
Basically, in a MVC terminology EditorPreferences.setDiffSidebar controls the "view" while VersioningCache controls the "model".
>> Currently this is done inside ADS by calling VersioningCache.setBase(JEditorPane, original, base = null)
,
This statement is incorrect. We don't currently do this. Which means this is not a replacement, it is an addition.
The addition should be added to DatastudioOptions :
d = new OptionDialog2(f, b.getRoot()) {
protected void onOk() {
}
Emilian, do you agree with this?
>> Currently this is done inside ADS by calling VersioningCache.setBase(JEditorPane, original, base = null)
,
This statement is incorrect. We don't currently do this. Which means this is not a replacement, it is an addition.
The addition should be added to DatastudioOptions :
d = new OptionDialog2(f, b.getRoot()) {
protected void onOk() {
}
Emilian, do you agree with this?
Like I've said, in a MVC terminology EditorPreferences.setDiffSidebar controls the "view" while VersioningCache controls the "model".
Indeed, you should add a EditorPreferences.setDiffSidebar in the Options classes, in order to enable/disable it per mimetype.
To understand why this issue was posted you should look at #7631. There we had the same file opened twice with different mimetype and each mimetype had its own diff sidebar settings (one was visible, the other was not). So you would need to use both API to fix that bug.
Like I've said, in a MVC terminology EditorPreferences.setDiffSidebar controls the "view" while VersioningCache controls the "model".
Indeed, you should add a EditorPreferences.setDiffSidebar in the Options classes, in order to enable/disable it per mimetype.
To understand why this issue was posted you should look at #7631. There we had the same file opened twice with different mimetype and each mimetype had its own diff sidebar settings (one was visible, the other was not). So you would need to use both API to fix that bug.
>> Currently this is done inside ADS by calling
VersioningCache.setBase(JEditorPane, original, base = null)
,This statement is incorrect. We don't currently do this. Which means this is not a replacement, it is an addition.
I've stated that thinking that the base parameter was cut (i.e. set to null for versioned files ! ) on purpose, to hide the changes on the Diff Sidebar.
For the same .txt versioned file, if I have the "Inline Diff Sidebar" option checked then I open the file in a new editor, I get the following call VersioningCache.setBase(JEditorPane, original, base != null)
, which is expected.
Now, if I uncheck the "Inline Diff Sidebar" and open the same versioned file again in a new editor, I get this call VersioningCache.setBase(JEditorPane, original, base == null)
, which indicates that it was cut somewhere (it is a versioned file!) by the state of the "Inline DIff Sidebar" option.
This could lead to another bug, because if the base parameter is null, the editor "model" handles this file as an unversioned file. If you proceed with the changes on the "view" as Emilian suggested above, when user enables the "Inline Diff Sidebar", it would operate on the "view" and make the diff sidebar visible for already opened editors with that mimetype (expected). But as the model (VersioningCache) does not have a base file for that editor, no diffs will show up for this editor.
Therefore, it is best to set base = null only for unversioned files and let the "Inline Diff Sidebar" option operate directly on the view.
>> Currently this is done inside ADS by calling
VersioningCache.setBase(JEditorPane, original, base = null)
,This statement is incorrect. We don't currently do this. Which means this is not a replacement, it is an addition.
I've stated that thinking that the base parameter was cut (i.e. set to null for versioned files ! ) on purpose, to hide the changes on the Diff Sidebar.
For the same .txt versioned file, if I have the "Inline Diff Sidebar" option checked then I open the file in a new editor, I get the following call VersioningCache.setBase(JEditorPane, original, base != null)
, which is expected.
Now, if I uncheck the "Inline Diff Sidebar" and open the same versioned file again in a new editor, I get this call VersioningCache.setBase(JEditorPane, original, base == null)
, which indicates that it was cut somewhere (it is a versioned file!) by the state of the "Inline DIff Sidebar" option.
This could lead to another bug, because if the base parameter is null, the editor "model" handles this file as an unversioned file. If you proceed with the changes on the "view" as Emilian suggested above, when user enables the "Inline Diff Sidebar", it would operate on the "view" and make the diff sidebar visible for already opened editors with that mimetype (expected). But as the model (VersioningCache) does not have a base file for that editor, no diffs will show up for this editor.
Therefore, it is best to set base = null only for unversioned files and let the "Inline Diff Sidebar" option operate directly on the view.
This API method should be called also when options are deserialized at application startup. Probably BaseEditorOptionSettings class is the best place to call it from.
This API method should be called also when options are deserialized at application startup. Probably BaseEditorOptionSettings class is the best place to call it from.
Issue #7644 |
Closed |
Fixed |
Resolved |
Completion |
No due date |
Fixed Build 12.0.0-rc-11 |
No time estimate |
1 issue link |
relates to #7631
Issue #7631Issues with Editor with a file under version control: Contents are deleted, Syntax Coloring, Inline Diff Sidebar |
This is part of the changes required to fix #7631.