Bug 558980 - Hook into Git History
Summary: Hook into Git History
Status: RESOLVED FIXED
Alias: None
Product: EGit
Classification: Technology
Component: UI (show other bugs)
Version: unspecified   Edit
Hardware: PC Windows 10
: P3 normal (vote)
Target Milestone: 5.7   Edit
Assignee: Project Inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-01-09 09:32 EST by Wim Jongman CLA
Modified: 2020-02-29 06:47 EST (History)
2 users (show)

See Also:


Attachments
Git History Interaction (84.65 KB, image/gif)
2020-01-09 09:32 EST, Wim Jongman CLA
no flags Details
Git History Page adaption class (4.22 KB, text/plain)
2020-02-24 10:56 EST, Evangelos Gkinis CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Wim Jongman CLA 2020-01-09 09:32:43 EST
Created attachment 281431 [details]
Git History Interaction

We have tasks that are associated with a branch. When the user selects a branch we want the git history view to react. We have it working (see attachment) but only by using an internal class GitHistoryPageSource.

Can we trigger the git history view without accessing internal classes? 

If not, would it be an idea to supply some factory class that can supply the GHPS instance?
Comment 1 Thomas Wolf CLA 2020-02-20 16:09:53 EST
I presume you mean HistoryPageInput? Can you show some code of what you do currently? Would make it easier to design a suitable public API...
Comment 2 Thomas Wolf CLA 2020-02-21 13:14:55 EST
Actually, after looking again at the GitHistoryPage code, it seems to me it should react if your view:

* Makes its selection available through a SelectionProvider (most likely that's the case already), and
* Your task objects (or whatever is selected there) adapt to Repository.
Comment 3 Wim Jongman CLA 2020-02-24 09:41:12 EST
(In reply to Thomas Wolf from comment #2)
> Actually, after looking again at the GitHistoryPage code, it seems to me it
> should react if your view:
> 
> * Makes its selection available through a SelectionProvider (most likely
> that's the case already), and
> * Your task objects (or whatever is selected there) adapt to Repository.

Yes, this is what we do and how we hoped it worked. It tuns out that history view was not reacting. We will dive in and supply an analysis.
Comment 4 Evangelos Gkinis CLA 2020-02-24 10:56:35 EST
Created attachment 281911 [details]
Git History Page adaption class
Comment 5 Evangelos Gkinis CLA 2020-02-24 10:57:51 EST
I just debugged it without the IGitHistoryPageSource adaption and it wasn't working. The workflow is like this:

1) First the GenericHistoryView asks for a IGitHistoryPageSource (through adaption) in its getPageSourceFor method. This means that the view is asking if someone can provide content for it and the view shows only content of type IGitHistoryPageSource.

2) Then the page provided from the previous adaption is actually a GitHistoryPageSource and that one, in the method canShowHistoryFor, asks for a Repository adaption.

3) Lastly the GItHistoryPage, through its inputSet method, asks for RevCommit in order to select that specific commit (and scroll there to make it visible). If its not provided then the view will show the entire history of the repository from the start (no commit will be selected).

If there is no adaption for an IGitHistoryPageSource then its only the RepositorySourceProvider through its selectionChanged method that asks for a repository but the repository given is actually lost later because it has no IGitHistoryPageSource to put it in in order to be displayed on the view.

I would expect a IGitHistoryPageSource to be instantiated by default internally from the GenericHistoryView when the history view is first opened and then reuse that (since its a public INSTANCE field already in the GitHistoryPageSource class) to ask for repositories/commits when the selection changes but the way it works is that the GenericHistoryView keeps asking for a IGitHistoryPageSource every time upon selection changed.

I have attached the file with the code on how I do it.
Comment 6 Thomas Wolf CLA 2020-02-24 11:28:59 EST
(In reply to Evangelos Gkinis from comment #5)
> I have attached the file with the code on how I do it.

Right, I forgot about the adaptation to IHistoryPageSource. Can you try the following: instead of adapting in your own factory, add

   <extension
         point="org.eclipse.core.runtime.adapters">
      <factory
            adaptableType="YOUR TASK CLASS HERE"
            class="org.eclipse.egit.ui.internal.factories.GitAdapterFactory">
         <adapter type="org.eclipse.team.ui.history.IHistoryPageSource" />
      </factory>
   </extension

in your UI bundles plugin.xml? GitAdapterFactory actually adapts anything to an IHistoryPageSource, and always returns the singleton GitHistoryPageSource.

If it then works (with your task object adapting to Repository), EGit can simply provide a public factory (as opposed to an internal one) so that you don't depend on EGit internals.

The second of your problems apparently is that you want to specify exactly which commit or branch to show. I'm pretty sure we could solve that, too, with a minimal change in GitHistoryPage if your task object also adapts to RevCommit (or to ObjectId).

But let's first see if the above proposal already makes the history page show the correct repository when your task object is selected.
Comment 7 Evangelos Gkinis CLA 2020-02-25 05:22:48 EST
Thanks for the fast reply.

It works (now I have 2 adapter classes for the same task class) but still the GitAdapterFactory is also an internal class and the purpose is to avoid using internal classes if possible. If I have to use internal classes then it doesn't matter which one it is in order to get the GitHistoryPageSource instance.

How to adapt to a Repository or RevCommit is not a problem cause neither are internal classes and I already adapt my task class to them.

What I don't understand is why the view requires an IGitHistoryPageSource all the time. I would expect it to ask through adaption if someone wants to provide his own and if no one can adapt to it then it should use the default instance. I may be wrong because I don't know the eGit API so well and any internal constraints it might have but thats how I see it as someone out of the box.
Comment 8 Thomas Wolf CLA 2020-02-25 06:19:23 EST
(In reply to Evangelos Gkinis from comment #7)
> It works (now I have 2 adapter classes for the same task class) but still
> the GitAdapterFactory is also an internal class and the purpose is to avoid
> using internal classes if possible. If I have to use internal classes then
> it doesn't matter which one it is in order to get the GitHistoryPageSource
> instance.
I *wrote* if that worked we could provide a public factory for this, didn't I?

And frankly said, I'd prefer using the GitAdapterFactory over that reflective hack in attachment 281911 [details] at any time. :-)

> What I don't understand is why the view requires an IGitHistoryPageSource
> all the time. I would expect it to ask through adaption if someone wants to
> provide his own and if no one can adapt to it then it should use the default
> instance. I may be wrong because I don't know the eGit API so well and any
> internal constraints it might have but thats how I see it as someone out of
> the box.
This is not EGit, it's the general Eclipse Team framework. The GenericHistoryPage tries to get an IHistoryPageSource through adaptation, and then asks that one whether it can show a history for the item. If there's no IHistoryPageSource, there's nothing that could show a history at all. If you had other tasks that map to CVS repositories, you'd adapt those to some CVS IHistoryPageSource to make the CVS team provider provide its history page.
Comment 9 Evangelos Gkinis CLA 2020-02-25 06:49:09 EST
I 'm sorry I am quite tired and forgot you said that. You are right about the team framework as well as the GenericHistoryVIew is from the team bundle.

A factory returning the instance would be ok then. Thanks for all your attention.
Comment 10 Eclipse Genie CLA 2020-02-25 07:04:58 EST
New Gerrit change created: https://git.eclipse.org/r/158297
Comment 11 Eclipse Genie CLA 2020-02-29 06:15:58 EST
Gerrit change https://git.eclipse.org/r/158297 was merged to [master].
Commit: http://git.eclipse.org/c/egit/egit.git/commit/?id=f826c57f208566e96f05c7902d58be90c03ad76b
Comment 12 Thomas Wolf CLA 2020-02-29 06:20:06 EST
New public adapter factory org.eclipse.egit.ui.history.GitHistoryAdapterFactory.

See https://wiki.eclipse.org/EGit/New_and_Noteworthy/5.7#API .
Comment 13 Wim Jongman CLA 2020-02-29 06:47:31 EST
Very nice. Thank you Thomas.