Community
Participate
Working Groups
Git Version : Eclipse EGit Git integration for Eclipse 6.0.0.202111291000-r org.eclipse.egit Eclipse Version - 4.23.0.v20211222-1800 The Tables are not aligned in the top and left for Git History and Git pull view. Attaching the image for History view. Same issue with Egit 5.12 version and Eclipse Version - 4.23.0.v20211222-1800
Created attachment 287725 [details] Egit History view
I don't see this in Eclipse 4.22 on OS X 10.14.6. Do you see this on Eclipse 4.22, too? If not, I would assume it's a problem in platform 4.23, not in EGit. If you also have this in Eclipse 4.22: which OS X version? There have been no changes at all in EGit regarding the layout of this view. Perhaps something has changed in JFace or SWT/Mac? Or in Cocoa, depending on your OS X version.
Moving to Platform, I was not seeing this till Dec 14th Build of 4.23. I am using 10.15.7 MacOSX. Moving to Platform UI for comment.
Can reproduce with I20211215-1800. Looks like a regression from the fix of Bug 577767.
(In reply to Sarika Sinha from comment #4) > Can reproduce with I20211215-1800. > Looks like a regression from the fix of Bug 577767. @Alexander, could you please take a look?
This is caused by patch for Bug 575259 instead of what was said before. That's also my patch, I'll investigate.
The problem occurs due to `org.eclipse.jface.layout.AbstractColumnLayout`. My patch sets initial location to the Table to work around a problem in macOS. The expectation is that any reasonable application would set its own position to the Table after creating it. However, `AbstractColumnLayout` just doesn't. It only ever calls `setSize()` to set the size, and location remains untouched. I would says that this is a bug in `AbstractColumnLayout` where it expects that initial location would match what it desires. I understand that this is not documented anywhere and it is incorrect to expect any specific location without setting it. [1] https://git.eclipse.org/c/platform/eclipse.platform.ui.git/tree/bundles/org.eclipse.jface/src/org/eclipse/jface/layout/AbstractColumnLayout.java
My suggestion is to revert the patch till JFace is fixed, as it is really difficult to work with Eclipse currently.
I would prefer that JFace is fixed instead. Afterall this is where the problem is. It requires a trivial fix and I fear that if patch is reverted, it will stay this way for a long time.
To my understanding, the fix is to replace all scrollable.setSize(w, h) in `org.eclipse.jface.layout.AbstractColumnLayout` with scrollable.setBounds(0, 0, w, h) Is this Bug even seen by the people who can fix it?
(In reply to Alexandr Miloslavskiy from comment #10) > To my understanding, the fix is to replace all > scrollable.setSize(w, h) > > in `org.eclipse.jface.layout.AbstractColumnLayout` > with > scrollable.setBounds(0, 0, w, h) Feel free to provide a patch :) > Is this Bug even seen by the people who can fix it? Not sure what you mean. If you have a fix, please push to gerrit. Or do you mean, the code that need to be changed is not inside Eclipse.org repositories?
Unfortunately I'm not skilled to build Eclipse, so I can only provide a patch without any testing at all. So someone will need to test it (on macOS)
(In reply to Alexandr Miloslavskiy from comment #12) > Unfortunately I'm not skilled to build Eclipse, so I can only provide a > patch without any testing at all. So someone will need to test it (on macOS) You do not need to build anything, just starting Eclipse from Eclipse with checked out projects will be enough - Eclipse will pick up classes from the workspace projects. If you have a patch in mind, please push to gerrit. I have no Mac either, but there are committers using Mac here.
I think it is better to revert Bug 577767 for now and reapply once M1 is done.
New Gerrit change created: https://git.eclipse.org/r/c/platform/eclipse.platform.ui/+/189243
I have submitted a patch without testing. Can someone try it? (In reply to Andrey Loskutov from comment #13) > You do not need to build anything, just starting Eclipse from Eclipse with > checked out projects will be enough - Eclipse will pick up classes from the > workspace projects. If you have a patch in mind, please push to gerrit. I > have no Mac either, but there are committers using Mac here. I bet it's slightly more complicated and I'm completely new with this. How do I start Eclipse from Eclipse? Do I need to build project before starting? And I think I'll also need to hand-edit some .classpath files after checking out? Is there some guide already available?
(In reply to Alexandr Miloslavskiy from comment #16) > I bet it's slightly more complicated and I'm completely new with this. How > do I start Eclipse from Eclipse? > Do I need to build project before starting? No, auto-build should be enabled by default, so after any code change + save everything is built. > And I think I'll also need to hand-edit some .classpath files after checking > out? *Only* for SWT project you have to rename existing platform dependent .classpath files to .classpath, e.g. from .classpath_cocoa to .classpath if you are on Mac etc. Other projects like JFace need to be imported from git into the workspace without any additional setup. > Is there some guide already available? https://wiki.eclipse.org/Platform/How_to_Contribute TL;DR: Grab latest SDK build for you platform like https://download.eclipse.org/eclipse/downloads/drops4/I20220103-1800/ Unzip & start Eclipse with new workspace. Import code of the project you want like JFace into the workspace. Change and save the code you need in Eclipse (will be built automatically). Go to Run -> Debug Configurations, double click on "Eclipse Application" to create new launch configuration and click on "Debug" button. You should be able now to see your changed code inside started Eclipse IDE.
Thanks for the explanation! I hope to find time to try it later today.
(In reply to Alexandr Miloslavskiy from comment #18) > Thanks for the explanation! I hope to find time to try it later today. Thanks, We can release your changes with SWT one for M2. We can not release M1 with current broken state.
Judging from the discussion in Gerrit, my patch for jface worked.\ Either way, M2 is also fine with me, I only worried about putting the patch away for many months.
(In reply to Alexandr Miloslavskiy from comment #20) > Judging from the discussion in Gerrit, my patch for jface worked.\ > > Either way, M2 is also fine with me, I only worried about putting the patch > away for many months. Your patch can be released on Friday 7th Jan after M1 goes out.
I've reverted Bug 575259 for M1 and triggered a new I-build.
Fixed in I20220104-0940.
Should we leave this bug open for JFace fix or will be handled in Bug 575259?
(In reply to Alexandr Miloslavskiy from comment #16) > I have submitted a patch without testing. Can someone try it? > > (In reply to Andrey Loskutov from comment #13) > > You do not need to build anything, just starting Eclipse from Eclipse with > > checked out projects will be enough - Eclipse will pick up classes from the > > workspace projects. If you have a patch in mind, please push to gerrit. I > > have no Mac either, but there are committers using Mac here. > > I bet it's slightly more complicated and I'm completely new with this. How > do I start Eclipse from Eclipse? Do I need to build project before starting? > And I think I'll also need to hand-edit some .classpath files after checking > out? Is there some guide already available? Happy new year. For platform ui it is simple. You import the project into Eclipse via File -> Open Projects from File System. Afterwards Eclipse default setting will build the project automaticallly with no additional setup required and you can start a runtime Eclipse with right-mouse click Run As -> Eclipse Application. This may require a fresh I-Build from https://download.eclipse.org/eclipse/downloads/ in case you want to use latest API. You may see an error for the API baseline, which you can turn into a warning via CTRL+1 on the error. More infos: https://www.vogella.com/tutorials/EclipsePlatformDevelopment/article.html#exercise-configuring-the-api-baseline
(In reply to Sarika Sinha from comment #24) > Should we leave this bug open for JFace fix or will be handled in Bug 575259? Let's keep it open for JFace fix.
I have now tried debugging Eclipse. Thanks for your hints on how to do that! I've ran into multiple difficulties :( Here's what I did: 1) Downloaded Eclipse IBuild [1] 2) Cloned eclipse.platform.ui.git [2] 3) Installed plugins [3] mentioned in guide [4] 4) Created Debug Configuration with 'Eclipse Application' 5) Configured API baseline to downloaded Eclipse I have 762 build errors. Some are there because it tries to build other platforms (win32, gtk) for some reason, such as: "OleClientSite cannot be resolved to a type". Some seem to be because tests require some other plugins or something, like "Bundle 'org.eclipse.core.tests.harness' cannot be resolved". I "resolved" these by deleting all subprojects except 'org.eclipse.jface'. Then, I had error "Unresolved requirement: Require-Bundle: org.eclipse.jface; bundle-version="[3.18.0,4.0.0)"". I understand that this is because the patch changes version of jface, because otherwise build server refuses to accept the patch. I "resolved" this by discarding part of my patch that changed versions. Eventually, I was able to debug jface alone and confirm that my patch works. What did I do wrong? [1] https://download.eclipse.org/eclipse/downloads/drops4/I20220103-1800/download.php?dropFile=eclipse-SDK-I20220103-1800-macosx-cocoa-x86_64.dmg [2] https://git.eclipse.org/r/platform/eclipse.platform.ui.git [3] http://git.eclipse.org/c/platform/eclipse.platform.ui.git/plain/releng/org.eclipse.ui.releng/platformUiTools.p2f [4] https://wiki.eclipse.org/Platform/How_to_Contribute
(In reply to Alexandr Miloslavskiy from comment #27) > I have now tried debugging Eclipse. Thanks for your hints on how to do that! > > I've ran into multiple difficulties :( > > Some are there because it tries to build other platforms (win32, gtk) for > some reason, such as: "OleClientSite cannot be resolved to a type". You can close these projects, e.g. right-mouse click -> Close. Deleting them is also fine. I added the closing to my setup description, see https://www.vogella.com/tutorials/EclipsePlatformDevelopment/article.html#closing-projects > Some > seem to be because tests require some other plugins or something, like > "Bundle 'org.eclipse.core.tests.harness' cannot be resolved". These require the bundles from the test feature. I added it to my setup description, see https://www.vogella.com/tutorials/EclipsePlatformDevelopment/article.html#install-test-feature > Then, I had error "Unresolved requirement: Require-Bundle: > org.eclipse.jface; bundle-version="[3.18.0,4.0.0)"". I understand that this > is because the patch changes version of jface, because otherwise build > server refuses to accept the patch. I "resolved" this by discarding part of > my patch that changed versions. That is strange. Which plug-in did give you this error? > Eventually, I was able to debug jface alone and confirm that my patch works. For SWT Java code you would also need to copy the .classpath_YOUROS file to .classpath. Afterwards it should also just work. For the native bits I don't know how this works. Package Explorer / Project Explorer filter the . files, so you would have to remove the filter to see them in Eclipse for example via filter button in the toolbar of the Project Explorer.
Thanks for clarifications!
I can see that M1 is published. Please apply my jface patch and restore my patch for Bug 575259
(In reply to Alexandr Miloslavskiy from comment #30) > I can see that M1 is published. Please apply my jface patch and restore my > patch for Bug 575259 I tried this locally in my workspace - Restored patch for Bug 575259 and applied the jface patch. I can still see some Tables in the preferences page are misaligned. Preferences > Java > Code Style Preferences > Java > Editor > Templates Preferences > Ant > Editor > Templates May be they are not using AbstractColumnLayout?
I'll check that within a few hours.
New Gerrit change created: https://git.eclipse.org/r/c/jdt/eclipse.jdt.ui/+/189438
New Gerrit change created: https://git.eclipse.org/r/c/platform/eclipse.platform.text/+/189439
Apparently the ancient technology of copy&pasting bugs was used in Eclipse :) I have added patches for 2 more projects which had a copy of buggy code. This fixes all problems mentioned so far.
Gerrit change https://git.eclipse.org/r/c/platform/eclipse.platform.text/+/189439 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.text.git/commit/?id=58fbfc9fd2266016e490fd06a2c975017966e2d7
(In reply to Lars Vogel from comment #28) > That is strange. Which plug-in did give you this error? Curiously this no longer happens on my second attempt (after cherry picking the original patch again). I'll forget it for now as single-time problem, I think.
Gerrit change https://git.eclipse.org/r/c/platform/eclipse.platform.ui/+/189243 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=d96542119b431622fd8fc732282d434f31be9006
(In reply to Alexandr Miloslavskiy from comment #35) > Apparently the ancient technology of copy&pasting bugs was used in Eclipse > :) I have added patches for 2 more projects which had a copy of buggy code. > This fixes all problems mentioned so far. Thanks for the patches, Alexandr! I've verified that it fixes the problems in the preferences. However, this also highlights that the change in SWT could potentially break other client and layout code that has similar patterns and I'm not sure if we have/can covered all the cases.
I understand why you are concerned. My thinking is that the bug is in fact rather unlikely - it's quite weird to never set Control's position. In Eclipse, the bug slipped through somehow and then it simply happened to be copy&pasted to other places. I think that it makes sense to merge the patch and see what happens. If other similar bugs pop up, they are easy to fix. In worst case, the patch can be reverted again.
(In reply to Alexandr Miloslavskiy from comment #40) > I understand why you are concerned. > > My thinking is that the bug is in fact rather unlikely - it's quite weird to > never set Control's position. In Eclipse, the bug slipped through somehow > and then it simply happened to be copy&pasted to other places. > > I think that it makes sense to merge the patch and see what happens. If > other similar bugs pop up, they are easy to fix. In worst case, the patch > can be reverted again. What if the client projects( Not in Platform or JDT which we can fix) have made same mistake and they are in maintenance?
(In reply to Alexandr Miloslavskiy from comment #40) > I understand why you are concerned. > > My thinking is that the bug is in fact rather unlikely - it's quite weird to > never set Control's position. In Eclipse, the bug slipped through somehow > and then it simply happened to be copy&pasted to other places. > Looking at this further, the problems seen in Eclipse were caused due to the unexpected change in SWT Table and Tree's default location (getLocation()) Please see the SWT snippet and description in Bug 575259 comment 13. Thank you for the time and effort that went into fixing the problems seen in Eclipse. But, this SWT behaviour change would break other SWT clients as well, so we should fix it in SWT.
From comment#23 and comment#24, the issue is fixed by reverting the change in SWT. Closing bug.
