Bug 552838 - Allow EGit use of external diftools defined in .gitattributes
Summary: Allow EGit use of external diftools defined in .gitattributes
Status: RESOLVED FIXED
Alias: None
Product: EGit
Classification: Technology
Component: UI (show other bugs)
Version: 5.6   Edit
Hardware: PC All
: P3 enhancement (vote)
Target Milestone: 6.2   Edit
Assignee: Andrey Loskutov CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on: 356832 552338 552840
Blocks:
  Show dependency tree
 
Reported: 2019-11-08 08:32 EST by Mykola Zakharchuk CLA
Modified: 2022-06-07 07:38 EDT (History)
4 users (show)

See Also:


Attachments
Prototype screen shot (49.99 KB, image/png)
2019-11-13 07:52 EST, Mykola Zakharchuk CLA
no flags Details
Updated prototype screen shot (73.06 KB, image/png)
2020-01-13 04:52 EST, Mykola Zakharchuk CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Mykola Zakharchuk CLA 2019-11-08 08:32:38 EST
External Diff-/Mergetools Preferences Page should allow using custom difftools defined in .gitattributes.
Comment 1 Mykola Zakharchuk CLA 2019-11-13 07:47:21 EST
After merging external diff support via bug 356832 and bug 552338 we can use external difftool defined in Eclipse preferences to execute diff for every data type. However, we want to teach git using external difftool for some particular data type. This makes sense in a case when Eclipse compare is not capable to offer functionality that end-user needs (e.g comparison of excel sheet/word document files). Means, ideally Eclipse "knows" which tool to use for which file type - for text files, default tooling, for .doc files some 3rd party diff/merge tool.

We think that we have following use cases for JGit/EGit diff/merge for a particular data type (prototype screenshot is coming):

1) Use Built-in Eclipse compare editor to execute diff with an option to use (external) tool specified in git attributes (checkbox "If configured by git, use external tool"). This option should be set per default and allows end-user to use the standard approach to execute diff (with compare tool) and triggers the execution of an external difftool, if one explicitly specified external tool in git attributes. If the checkbox is disabled - use Eclipse compare for everything (default Eclipse behavior).

2) Use external difftool, trust git (.gitconfig and .gitattributes). This option relies on the system configuration of git (.gitconfig, .gitattributes) and obeys the general rules of git configs. Git attributes wins over default difftool specified by gitconfig.

3) External (specified) tool. End-user specifies which tool he wants to use from the list (provided by git configuration). We force Eclipse and JGit to use this specified external difftool for every diff execution, independently of what is specified by git attributes / git config.

The Merge section should be also moved from the general Git preferences page to the new Diff/Merge preference page.
Comment 2 Mykola Zakharchuk CLA 2019-11-13 07:52:44 EST
Created attachment 280622 [details]
Prototype screen shot
Comment 3 Eclipse Genie CLA 2019-12-05 10:35:18 EST
New Gerrit change created: https://git.eclipse.org/r/153907
Comment 4 Mykola Zakharchuk CLA 2020-01-13 04:38:15 EST
To use external diff tool for specific file type one should:

- Specify an external tool in config. file and define the parameters the tool awaits:
[difftool "customTool"]
   cmd = /home/user/diff_tool.sh - $LOCAL - - $REMOTE

- Define a file type and the corresponding custom diff tool to execute diff in git attributes:
 *.txt 		diff=customTool
Comment 5 Mykola Zakharchuk CLA 2020-01-13 04:48:46 EST
The line in git attributes should look like this:
 *.txt         difftool=customTool

Please ignore the example in the previous post.
Comment 6 Mykola Zakharchuk CLA 2020-01-13 04:52:15 EST
Created attachment 281460 [details]
Updated prototype screen shot

The updated version of the new Diff/Merge preference page.
Comment 7 Andre Bossert CLA 2020-01-13 07:58:09 EST
That looks a way better in own page and with well described options!
Comment 8 Andre Bossert CLA 2020-01-13 08:02:25 EST
Good work! It should be merged together or short after the base patches are merged...
Comment 9 Simeon Andreev CLA 2020-02-04 10:45:59 EST
It would also be great if external tools can be configured with preferences. So that users are not expected to change their git configuration/attributes, for a product where some types of diffs should be "built-in". E.g. in our case, our product supports .ods file to some extent and users would like to diff such files (which are from Eclipse/git perspective just binaries).

We'll create a review with our suggested change, either amending the current change in this ticket or building on top of it (likely the latter). We'll do so as soon as we have validated the change in our product.
Comment 10 Eclipse Genie CLA 2020-03-02 09:20:26 EST
New Gerrit change created: https://git.eclipse.org/r/158679
Comment 11 Andrey Loskutov CLA 2020-03-02 09:21:16 EST
(In reply to Simeon Andreev from comment #9)
> It would also be great if external tools can be configured with preferences.
> So that users are not expected to change their git configuration/attributes,
> for a product where some types of diffs should be "built-in". E.g. in our
> case, our product supports .ods file to some extent and users would like to
> diff such files (which are from Eclipse/git perspective just binaries).
> 
> We'll create a review with our suggested change, either amending the current
> change in this ticket or building on top of it (likely the latter). We'll do
> so as soon as we have validated the change in our product.

That is https://git.eclipse.org/r/158679
Comment 12 Simeon Andreev CLA 2022-06-03 06:56:11 EDT
I'm trying to write a new test, for this I mimic this test: org.eclipse.egit.ui.test.team.actions.MergeToolTest

Unfortunately the SWT bot fails (when executed either as plug-in test or as an SWT bot test).

!ENTRY org.eclipse.ui 4 0 2022-06-03 12:51:38.607
!MESSAGE Unhandled event loop exception
!STACK 0
java.lang.LinkageError: loader constraint violation: when resolving method 'org.hamcrest.Matcher org.eclipse.swtbot.swt.finder.matchers.WidgetMatcherFactory.withMnemonic(java.lang.String)' the class loader org.eclipse.osgi.internal.loader.EquinoxClassLoader @77c73c34 of the current class, org/eclipse/egit/ui/test/ContextMenuHelper, and the class loader org.eclipse.osgi.internal.loader.EquinoxClassLoader @50abe570 for the method's defining class, org/eclipse/swtbot/swt/finder/matchers/WidgetMatcherFactory, have different Class objects for the type org/hamcrest/Matcher used in the signature (org.eclipse.egit.ui.test.ContextMenuHelper is in unnamed module of loader org.eclipse.osgi.internal.loader.EquinoxClassLoader @77c73c34, parent loader 'platform'; org.eclipse.swtbot.swt.finder.matchers.WidgetMatcherFactory is in unnamed module of loader org.eclipse.osgi.internal.loader.EquinoxClassLoader @50abe570, parent loader 'platform')
	at org.eclipse.egit.ui.test.ContextMenuHelper.getMenuItem(ContextMenuHelper.java:172)
	at org.eclipse.egit.ui.test.ContextMenuHelper$1.run(ContextMenuHelper.java:123)
	at org.eclipse.egit.ui.test.ContextMenuHelper$1.run(ContextMenuHelper.java:1)
	at org.eclipse.swtbot.swt.finder.finders.UIThreadRunnable$2.doRun(UIThreadRunnable.java:142)
	at org.eclipse.swtbot.swt.finder.finders.UIThreadRunnable$1.run(UIThreadRunnable.java:90)
	at org.eclipse.swt.widgets.RunnableLock.run(RunnableLock.java:40)
	at org.eclipse.swt.widgets.Synchronizer.runAsyncMessages(Synchronizer.java:132)
	at org.eclipse.swt.widgets.Display.runAsyncMessages(Display.java:5001)
	at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:4481)
	at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine$5.run(PartRenderingEngine.java:1155)
	at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:338)
	at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine.run(PartRenderingEngine.java:1046)
	at org.eclipse.e4.ui.internal.workbench.E4Workbench.createAndRunUI(E4Workbench.java:155)
	at org.eclipse.ui.internal.Workbench.lambda$3(Workbench.java:644)
	at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:338)
	at org.eclipse.ui.internal.Workbench.createAndRunWorkbench(Workbench.java:551)
	at org.eclipse.ui.PlatformUI.createAndRunWorkbench(PlatformUI.java:156)
	at org.eclipse.ui.internal.ide.application.IDEApplication.start(IDEApplication.java:152)
	at org.eclipse.swtbot.eclipse.core.UITestApplication.runApp(UITestApplication.java:63)
	at org.eclipse.swtbot.eclipse.core.UITestApplication.start(UITestApplication.java:58)
	at org.eclipse.equinox.internal.app.EclipseAppHandle.run(EclipseAppHandle.java:203)
	at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.runApplication(EclipseAppLauncher.java:136)
	at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.start(EclipseAppLauncher.java:104)
	at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:402)
	at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:255)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:566)
	at org.eclipse.equinox.launcher.Main.invokeFramework(Main.java:659)
	at org.eclipse.equinox.launcher.Main.basicRun(Main.java:596)
	at org.eclipse.equinox.launcher.Main.run(Main.java:1467)
	at org.eclipse.equinox.launcher.Main.main(Main.java:1440)

I've not ran SWT bot tests, so I need to install some extra bundles?
Comment 13 Simeon Andreev CLA 2022-06-03 08:33:01 EDT
(In reply to Simeon Andreev from comment #12)
> I'm trying to write a new test, for this I mimic this test:
> org.eclipse.egit.ui.test.team.actions.MergeToolTest
> 
> Unfortunately the SWT bot fails (when executed either as plug-in test or as
> an SWT bot test).

Opened bug 580076. Looks like there is a version conflict of 2x org.hamcrest bundles.
Comment 16 Eclipse Genie CLA 2022-06-06 09:07:26 EDT
New Gerrit change created: https://git.eclipse.org/r/c/egit/egit/+/193946