Community
Participate
Working Groups
N20060426-0010 Check Window > Preferences > Team > "Show the file author in compare editor" and then click on a file in the Synchronize view ==> only right side shows author information (see attached picture).
Created attachment 39638 [details] Picture showing missing author information
You can see the other author by revealing the ancestor pane. The problem here is that the local may be modified. What authgor should we show in tat case? The authore of the base or the local user?
Well, the same argument holds for the revision, right? And since the revision is shown I think it is clear to where the author belongs.
Perhaps. I have verified that the current behavior is the same as it was in 3.1. Since it is not a refgression, we will defer the issue to a later release.
Having the preference on and using N20071008-0010 I'm not able to locate the author on either left (local) nor right side of the compare editor.
More details about the issue can be found in bug 80577, I would even mark it as a duplicate.
>More details about the issue can be found in bug 80577, I would even mark it as >a duplicate. They are not really duplicates (at least not from a user point of view - didn't look at your code to check whether it really boils down to the same PR). The big difference is that a comparison opened via Sync view never shows the author on the left side: 1. make a change and save 2. click on the outgoing change in the Sync view ==> no author on the left side The other bug is about missing the author on both sides using the Repo view
Created attachment 119252 [details] Patch_v01 Tom, please comment on the patch.
Tomasz, the patch provided doesn't consider all the cases. Please hold on with the review until I fix it.
Looking forward to see this fixed ;-)
I'd like to finish this. >Tomasz, the patch provided doesn't consider all the cases. Pawel, can you tell me which cases you think are missing?
Those might be possible candidates but I don't think local resources are involved here (at least I could not trigger a test case in the UI): CVSParticipant.updateLabelsForCVS(...) CVSCompareEditorInput.getLabel(...) ResourceCompareInputChangeNotifier.CompareInputLabelProvider For now I'd like to commit a polished version of your patch (fixes some possible NPEs, a little bit of performance tuning, bundle version and @since tag updates). I think we can raise new bugs if we find remaining issues.
Created attachment 174837 [details] Fix
(In reply to comment #11) > I'd like to finish this. > > >Tomasz, the patch provided doesn't consider all the cases. > Pawel, can you tell me which cases you think are missing? Just sync a project and open a comparison from the Sync view.
>Just sync a project and open a comparison from the Sync view. Indeed. This would have to be fixed in ResourceCompareInputChangeNotifier.CompareInputLabelProvider It looks like for every different scenario to open a compare editor there are different editor inputs :-( Using Compare Current with x.y in the History view also doesn't fill in the author. It even doesn't fill in the revision of the local file. This would have to be fixed in CompareFileRevisionEditorInput. To proceed we have to add getAuthor() to LocalResourceTypedElement which is used in both of the above cases. Tomasz or Pawel, is there an easy way to get or create the SyncInfo for the local resource and then use SyncInfo.getLocalAuthor(IProgressMonitor) that we add with the patch? Or is there even a simpler way?
(In reply to comment #15) > It looks like for every different scenario to open a compare editor there are > different editor inputs :-( Welcome to our world. > Tomasz or Pawel, is there an easy way to get or create the SyncInfo for the > local resource and then use SyncInfo.getLocalAuthor(IProgressMonitor) that we > add with the patch? Or is there even a simpler way? The SyncInfo doesn't have the getLocalAuthor method you mentioned. Is this something you would like to add as well? My first impression looking at the related code is that we should try to get an implementation of IFileRevision in LocalResourceTypedElement. Have you considered this? The IFileRevision interface has a handy getAuthor() method.
Did you read my previous comment at all? ;-) >The SyncInfo doesn't have the getLocalAuthor method you mentioned From previous comment: This is part of the currently attached patch (Fix). >LocalResourceTypedElement. Have you considered this? From previous comment: To proceed we have to add getAuthor() to LocalResourceTypedElement Yes, I tried to implement getAuthor() using the getLocalAuthor() from the patch but got no luck. Maybe we can move the whole code that's currently (in patch) in SyncInfo.getLocalAuthor() into LocalResourceTypedElement but I didn't check whether that also covers the case from the patch.
(In reply to comment #17) > Did you read my previous comment at all? ;-) Oops, my cheap trick didn't work :) Seriously, I haven't noticed the patch you submitted. Apologies.
>Seriously, I haven't noticed the patch you submitted. Apologies. It's Pawel's patch with some enhancements.
(In reply to comment #15) > Tomasz or Pawel, is there an easy way to get or create the SyncInfo for the > local resource and then use SyncInfo.getLocalAuthor(IProgressMonitor) that we > add with the patch? CVSWorkspaceRoot.getCVSResourceFor(resource).getSyncInfo(). Dani, I've applied the latest patch and check the following scenarios: * Compare with > Latest from HEAD, authors on both sides * Non-model Sync View: Open in Compare Editor, authors on both sides * Model Sync View: Open in Compare Editor, author on the left side is missing * Repo View: Compare, authors on both sides * History View: comparing two remote revs, authors on both sides * History View: comparing Current with x.y, author on the left side is missing * History View: comparing remote rev with local, n/a (at least it current, but that's the case above) * History View: comparing local revs, n/a * Compare With > Each Other, n/a? do you want to display authors here as well? of course, only when comparing shared files Did I miss anything? I skipped comparisons made in dialogs, I treat them as obsolete.
>Did I miss anything? See comment 14.
I'm sorry, but isn't "sync a project and open a comparison from the Sync view" (comment 14) the same as "Model Sync View: Open in Compare Editor" (comment 20)?
>I'm sorry, but isn't "sync a project and open a comparison from the Sync view" >(comment 14) the same as "Model Sync View: Open in Compare Editor" (comment >20)? Looks like they are not. At least when you open the compare editor from the Sync view it's missing the left author (with the patch) as a different editor input is used.
Can you reproduce the missing author on the left? >CVSWorkspaceRoot.getCVSResourceFor(resource).getSyncInfo(). That does not return an org.eclipse.team.core.synchronize.SyncInfo. I have a working solution in place (see latest patch) but fetching the workspace author takes quite some time. I use this code: RepositoryProvider provider= RepositoryProvider.getProvider(getResource().getProject()); IFileRevision revision= provider.getFileHistoryProvider().getWorkspaceFileRevision(getResource()); revision= revision.withAllProperties(monitor); author= revision.getAuthor(); This call: revision.withAllProperties(monitor) is very slow. Any idea how we can get the local revision with filled properties faster? The code needs to be in Team not CVS.
Created attachment 176554 [details] Fix
>This call: revision.withAllProperties(monitor) is very slow. 1 to 2 seconds.
>>This call: revision.withAllProperties(monitor) is very slow. >1 to 2 seconds. OK, I did some more analyzes and tests and the originally proposed (and incomplete) patch from Pawel has the same issue but it's not noticed quickly because the whole editor fetches all its data and then shows everything together including the authors ==> opening of the editor gets slower. On the scenario via Synchronize view one sees the delay better because there the authors are updated in the background while the compare editor is already present. ==> Any fix can delay opening the compare editor via Compare With. To avoid this we would have to rework the code like it is done in the scenario where the editor is opened via Synchronize view but that's out of scope. However, I have improved the code to check the revision (content id) and avoid to fetch the author twice for the same revision (similar to the code in ResourceDiffCompareInput.propogateAuthorIfSameRevision(FileRevisionTypedElement, FileRevisionTypedElement). Also, I've improved the code in ResourceCompareInputChangeNotifier to only start one thread instead instead of one per left/right/ancestor. Remaining issue: the left author is still not shown for the case where one compares 'Current with x.y' in the History view. Can someone please review the patch and provide feedback about the direction? If it's good I can implement the missing scenario. Thanks.
Created attachment 176688 [details] Fix
Since I'm the only one in the outpost at the moment I suppose I'll be that "someone" :) So, here are my comments: * the fix doesn't work when opening comparison for an incoming addition (most probably because it sets fireEvent to false (was true) when no author is found for the left side[1]) * I guess you can use getLocalContentIdentifier() in getLocalAuthor()[2] if you want to get the localRevision * in the fetchAuthor[3] method you could use Utils.getHistoryProvider(getResource()) * you missed "c" in "@since 3.6" for getLocalAuthor[2] I haven't found anything else - other then the above, it works fine to me. [1] org.eclipse.team.internal.ui.mapping.ResourceDiffCompareInput.updateAuthorInfo(IProgressMonitor) [2] org.eclipse.team.internal.ccvs.core.CVSSyncInfo.getLocalAuthor(IProgressMonitor) [3] org.eclipse.team.internal.ui.synchronize.LocalResourceTypedElement.fetchAuthor(IProgressMonitor)
Created attachment 177328 [details] Fix >Since I'm the only one in the outpost at the moment I suppose I'll be that >"someone" :) Thanks! >* the fix doesn't work when opening comparison for an incoming addition (most >probably because it sets fireEvent to false (was true) when no author is found >for the left side[1]) Fixed. >* you missed "c" in "@since 3.6" for getLocalAuthor[2] Fixed. >* in the fetchAuthor[3] method you could use >Utils.getHistoryProvider(getResource()) Done. >* I guess you can use getLocalContentIdentifier() in getLocalAuthor()[2] if you >want to get the localRevision Not done. This is done on purpose for performance reasons. Committed this to HEAD.
Note that sometimes it shows "Local File 0" on the left side. This is has not been introduced by the fix, see bug 323509 for details.
>Using Compare Current with x.y in the History view also doesn't fill in the >author. Fixed that in HEAD (CompareFileRevisionEditorInput).
Verified for 3.7M2 on Linux with I20100913-1800.
