Community
Participate
Working Groups
When a text comparison is shown in the compare editor, the user should have the ability to set the encoding that is used to render the text (since the encoding provided by the compare input may be wrong). This is supported by Text editors by the Edit/Set Encoding... menu option. For compare editors, it is possible that the encoding for each of the contributors is different so it would be good if the Set Encoding action was available from the context menu of each text pane. However, it would be good if the dialog that was presented to the user gave them the option to set the encoding for either the target pane or all panes.
*** Bug 206586 has been marked as a duplicate of this bug. ***
Bug 206586 is a request for showing the detected (set) encoding for both panes of the compare view next to the revision/author.
*** Bug 209237 has been marked as a duplicate of this bug. ***
Bug 209237 shows why Michael's idea is worth considering.
Please provide a target milstone when this problem is going to be fixed.
(In reply to comment #5) > Please provide a target milstone when this problem is going to be fixed. At the moment we are working on adding some features available for editors but missing in Compare Editor (see bug 169386 and its blockers). This could be one of them as it seems to be related. However, I'm afraid we don't have the manpower to address your request at this time. Patches will be accepted and I'm always eager to help.
(In reply to comment #5) > Please provide a target milstone when this problem is going to be fixed. This would be fixed for M5.
One of the problems with fixing this bug is that for some comparisons TextMergeViewer doesn't operate on document providers. Bug 259940 has been opened to track this issue.
Created attachment 121626 [details] Patch_v01
Diff blocks/lines should be repainted when encoding is changed.
Moreover copyrights should be updated to 2009 and I noticed two new methods w/o since tag (API tooling marks it)
Created attachment 121790 [details] Patch_v02 Previous patch updated to HEAD, changes in bug 169386 made the patch hard to apply.
Created attachment 121791 [details] mylyn/context/zip
Created attachment 121805 [details] Patch_v03 Addressing comment 10 and comment 11 and some minor issues appointed by Tomasz.
(In reply to comment #10) > Diff blocks/lines should be repainted when encoding is changed. Recalculating diffs does happen but is faulty. Steps to reproduce: 1. Compare two local files, examine the diffs (both have encoding set to Cp1250 (or ISO-8859-1)) 2. Change the encoding for left side to UTF-18 3. Change the encoding back to the original one i.e. Cp1250. Observe that diffs has been updated but are inaccurate 4. Set the encoding to a compatible type (in terms of bit numbers) i.e. UTF-8. Diffs will be now correctly displayed. What I've observed while debugging is that in 3. the left doc taken for comparison has only one line and is half long comparing to the original, which suggests that wrong encoding is used (16 bits instead of 8).
Created attachment 121826 [details] Patch_v04 Addressing comment 15.
Created attachment 121932 [details] Patch_v05 I reused the ChangeEncodingAction from org.eclipse.ui.texteditor. This way no new API has to be introduce along with the fix. This also means that comment 15 is valid and has to be addressed in a separate bug.
The fix has to be back-ported to the maintanance stream. Setting target milestone to 3.4.2.
Created attachment 122084 [details] Patch_v06 Patch updates: - ChangeEncodingActions are properly updated ie disabled when the viewer is dirty - diffs are recalculated when the document has been replaced completely. Triggering events are: encoding change, complete file content replacement by copy-paste - some minor, cosmetic changes in the code Issues: - creating a ChangeEncodingAction requires internal message bundles, a public constructor with the strings set would be useful - the patch introduces a second version of an ITextEditor adapter (the first one already exists in MergeSourceViewer). Fixing bug 259410 will remove both adapters are replace them with an embedded text editor(s). However, since 3.4.x stream is not aware of changes made in bug 169386 (editors embedding), the ChangeEncodingAction released to that branch will use the adapter from the patch Pawel, could you take a look at the patch and confirm that it still works as expected?
Looks fine, thanks Tom.
> - creating a ChangeEncodingAction requires internal message bundles, a public > constructor with the strings set would be useful Filed bug 260525.
Patch_v06 is in HEAD. Waiting for PMC approval to release it to the 3.4.x stream.
Created attachment 122256 [details] Patch Two more things added to the previos patch: - move the piece of code responsible for calling to update diffs to elementContentReplaced method, no need to check if the recalculation is needed - use ChangeEncodingAction contructor with default labels
The latest patch has been also released to HEAD.
Created attachment 122258 [details] Patch_v06 for R3_4_maintenance branch I updated the patch to work with the 3.4.x branch. The problem is that in the meantime we decided to use different compare input when comparing local resources[1], which results in an AFE[2]. [1] bug 193324 [2] org.eclipse.core.runtime.AssertionFailedException: assertion failed: at org.eclipse.core.runtime.Assert.isTrue(Assert.java:111) at org.eclipse.core.runtime.Assert.isTrue(Assert.java:97) at org.eclipse.jface.preference.PreferenceStore.setValue(PreferenceStore.java:696) at org.eclipse.jface.preference.PreferenceStore.setDefault(PreferenceStore.java:524) at org.eclipse.ui.texteditor.ChangeEncodingAction$1.createDialogArea(ChangeEncodingAction.java:149) at org.eclipse.jface.dialogs.Dialog.createContents(Dialog.java:760) at org.eclipse.jface.window.Window.create(Window.java:431) at org.eclipse.jface.dialogs.Dialog.create(Dialog.java:1089) at org.eclipse.jface.window.Window.open(Window.java:790) at org.eclipse.ui.texteditor.ChangeEncodingAction.run(ChangeEncodingAction.java:200) at org.eclipse.jface.action.Action.runWithEvent(Action.java:498) at org.eclipse.jface.action.ActionContributionItem.handleWidgetSelection(ActionContributionItem.java:583) at org.eclipse.jface.action.ActionContributionItem.access$2(ActionContributionItem.java:500) at org.eclipse.jface.action.ActionContributionItem$5.handleEvent(ActionContributionItem.java:411) at org.eclipse.swt.widgets.EventTable.sendEvent(EventTable.java:84) at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1003) at org.eclipse.swt.widgets.Display.runDeferredEvents(Display.java:3823) at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:3422) at org.eclipse.ui.internal.Workbench.runEventLoop(Workbench.java:2382) at org.eclipse.ui.internal.Workbench.runUI(Workbench.java:2346) at org.eclipse.ui.internal.Workbench.access$4(Workbench.java:2198) at org.eclipse.ui.internal.Workbench$5.run(Workbench.java:493) at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:288) at org.eclipse.ui.internal.Workbench.createAndRunWorkbench(Workbench.java:488) at org.eclipse.ui.PlatformUI.createAndRunWorkbench(PlatformUI.java:149) at org.eclipse.ui.internal.ide.application.IDEApplication.start(IDEApplication.java:113) at org.eclipse.equinox.internal.app.EclipseAppHandle.run(EclipseAppHandle.java:193) at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.runApplication(EclipseAppLauncher.java:110) at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.start(EclipseAppLauncher.java:79) at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:386) at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:179) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(Unknown Source) at sun.reflect.DelegatingMethodAccessorImpl.invoke(Unknown Source) at java.lang.reflect.Method.invoke(Unknown Source) at org.eclipse.equinox.launcher.Main.invokeFramework(Main.java:549) at org.eclipse.equinox.launcher.Main.basicRun(Main.java:504) at org.eclipse.equinox.launcher.Main.run(Main.java:1236) at org.eclipse.equinox.launcher.Main.main(Main.java:1212)
Created attachment 122528 [details] Patch_v07 I addressed comment 25 and added structural differences rebuilding.
There are two issues related the fix in HEAD. One is bug 261025 which states that changing encoding in comapre editor doesn't trigger structured differences recalculation. The behaviour in 3.4.2 with Patch_v07 is correct though. The second one is 256805, this also has to be addressed only in the 3.5 development stream. The differences in fixes for 3.4.2 and 3.5 steam mainly from bug 193324.
(In reply to comment #27) Comment for 3.4.2 patch. When the encoding is changed in the compare editor, only part of the structure is rebuilt. AFAIK, the part connected to the viewer where the encoding is changed. I would always refresh whole structure. This change will prevent this case, steps: 1) Create a java file with UTF-8 encoding 2) Change the encoding to UTF-16 3) Open the compare editor for the file and its revision 4) Change the file encoding to UTF-8 in another editor 5) Change the revision encoding in the compare editor 6) Notice that the structure is rebuilt incorrectly Moreover, please update the date in ResourceEditionNode copyright.
Created attachment 122667 [details] 223857_v08_R3_4 Addressing comment 28.
Looks better now. I would add {@link} where class names are mentioned in javadocs. Otherwise looks good.
(In reply to comment #30) I noticed that _ENCODING constants in CompareConfiguration are new API. This has to be changed for 3.4.2.
Created attachment 122671 [details] 223857_v09_R3_4 Addressing comment 30 and comment 31.
Thanks for reviewing the code Szymon. Patch from the comment above has been released to 3.4.x branch.
