Add Managed Version Info on hover #238 #298
Conversation
|
Just a simple comment when you create a PR. I think it should be nice to add a little screenshot with th eresult of PR (or a little demo). It will be easier to understand what the PR provides and for history it is nice too. We do that for LemMinX and I find it is nice for the reviewer. It is just a suggestion that I do. |
| assertTrue(value.contains("The managed version is")); | ||
| assertTrue(value.contains("2.22.2")); | ||
| assertTrue(value.contains("The artifact is managed in")); | ||
| assertTrue(value.contains("org.apache.maven.plugins:maven-surefire-plugin:2.22.2")); |
There was a problem hiding this comment.
I don't think this should be added in when the version is the one that is defined in the pom. Managed version are only useful when no version is explicitly set (usually in the "child pom")
There was a problem hiding this comment.
But we still have to show the version and the location links on a pom where the version is clearly defined.
That was actually a problem as the method I've used for finding version in such case was returning a wrong version and link. So, I'd prefer this test case to also exist for parent pom.
There was a problem hiding this comment.
But we still have to show the version and the location links on a pom where the version is clearly defined.
we already have that I believe. Ctrl+Click can be used to get the link to the artifact.
...st/java/org/eclipse/lemminx/extensions/maven/participants/hover/ManagedVersionHoverTest.java
Outdated
Show resolved
Hide resolved
vrubezhny
force-pushed
the
fixManagedVersionInHover
branch
from
2e8c494 to
9ddaa97
Compare
| assertTrue(value.contains("The managed version is")); | ||
| assertTrue(value.contains("2.22.2")); | ||
| assertTrue(value.contains("The artifact is managed in")); | ||
| assertTrue(value.contains("org.apache.maven.plugins:maven-surefire-plugin:2.22.2")); |
There was a problem hiding this comment.
That's not correct. The artifact version is managed in pom-dependencyManagement-parent.
There was a problem hiding this comment.
Now a version from the parent pom and location of parent pom where the version is defined is shown in Hover.
If no version is specified in project it shows a version and location of pom for an according artifact resolved in maven project.
Is it OK to do it this way or I should not show any version in case it's not managed by project?
...st/java/org/eclipse/lemminx/extensions/maven/participants/hover/ManagedVersionHoverTest.java
Show resolved
Hide resolved
|
Any progress here? |
Signed-off-by: Victor Rubezhny <vrubezhny@redhat.com>
vrubezhny
force-pushed
the
fixManagedVersionInHover
branch
from
9ddaa97 to
b86c446
Compare
|
Thanks! |
|
Thank you! It would be great to also jump directly to the line where the given artifact version is defined and not to just open the linked pom.xml. A new feature request targeted on LemMinX, on m2e, or maybe both? |
Signed-off-by: Victor Rubezhny vrubezhny@redhat.com