-
Notifications
You must be signed in to change notification settings - Fork 226
Fix #3405 Updating code minings #3416
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Fix #3405 Updating code minings #3416
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot for the fixes, great work.
Could you please also create a unit test which documents the problem scenario where the CodeMiningType for a given offset is changed?
...org.eclipse.jface.text/src/org/eclipse/jface/internal/text/codemining/CodeMiningManager.java
Outdated
Show resolved
Hide resolved
...org.eclipse.jface.text/src/org/eclipse/jface/internal/text/codemining/CodeMiningManager.java
Outdated
Show resolved
Hide resolved
...lipse.jface.text.examples/src/org/eclipse/jface/text/examples/codemining/CodeMiningDemo.java
Outdated
Show resolved
Hide resolved
Hi @tobiasmelcher, I was thinking about how to implement a test for my case, but for some reason, some tests, esp. CodeMining test, fail on my machine on the master branch (without my changes), commit hash 0ce1430. I didn't find the reason yet. ![]() |
Thanks to feedback from @tobiasmelcher
Test Results 3 018 files 3 018 suites 2h 52m 43s ⏱️ Results for commit e85cb9d. ♻️ This comment has been updated with latest results. |
Avoid using AbstractCodeMining class since otherwise we cannot determine which of the following three cases we have: in-line code mining, line header code minings, or line footer code mining. See org.eclipse.jface.internal.text.codemining.CodeMiningManager.
Hi @tobiasmelcher and @BeckerWdf, I'm trying to implement a test for issue #3405, but I'm struggling with various issues with running the tests on my machine(s). I might need some help. Some tests fail for the unmodified Eclipse platform UI master branch, commit 0ce1430, on MacOS and on Windows 11 as well. Due to the failing tests, I cannot assure that my new test will behave as expected. In addition, the SWT widgets or the tests seem to behave strange. For example, when debugging my new test case on MacOS, the first line's vertical line indentation is reported to be 16, but the code mining is being drawn onto the source viewer's content instead of drawing it into an extra line (see screenshot 1). The in-line code mining is sometimes also drawn into the source viewer's text (see screeshot 2). The method I've extended the code mining demo where the code minings seem to behave as expected, i.e. drawing a "title" either above a reference (e.g. "REF-X" in my example) in a line header or in the same line in front of the referencing text: It would be great if someone could take a look at my PR draft and help me fixing the tests. |
@travkin79 sure, I'll take a look and make a proposal. Please give me some time. I will also check if I can reproduce the other test errors you have mentioned. |
8528945
to
8b1e2fd
Compare
8b1e2fd
to
e85cb9d
Compare
Thank you @tobiasmelcher, I managed to finish a first version of a test. It turns out, Besides that, there might be an issue with my PR in case that someone is using |
There are three types of code minings and three corresponding types of annotations. Issue #3405 describes a problem with updating the annotations. In certain cases the annotations are not re-created if the code mining type changes, e.g. from in-line code minings to line header code minings. This PR fixes that issue.
In order to make reviews easy, I left three commits for three different changes. The commits can be squashed if you prefer having only one commit.