Skip to content
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

Add AST view and Java Element view to SDK #387

Closed
iloveeclipse opened this issue Jan 15, 2023 · 35 comments · Fixed by #396 or #402
Closed

Add AST view and Java Element view to SDK #387

iloveeclipse opened this issue Jan 15, 2023 · 35 comments · Fixed by #396 or #402

Comments

@iloveeclipse
Copy link
Member

I've always wondered why such interesting JDT tools like AST view and Java Element view are not part of the SDK.
image.

The views offer insight about JDT internals, and are useful mostly for JDT development. But I think it would be beneficial also for few JDT developers and contributors to have such tools at hand.

I'm actually not aware how an average user can install them, I don't see them on any update site (or I'm blind).
The views themselves don't introduce any "unwanted" side effects in the IDE, so if someone isn't interested in them, they do not harm and do not disturb as not shown by default.

So why not add them to the SDK by default?

@akurtakov
Copy link
Contributor

As a first step I would say create a feature (or features) and publish them on the update site. After that they can be included in the SDK zip.

@vogella
Copy link
Contributor

vogella commented Jan 15, 2023

+1 for adding them to the SDK

@iloveeclipse
Copy link
Member Author

create a feature (or features)

I see features in JDT UI git repo.

and publish them on the update site.

May be that was broken at some time?
See for example https://github.com/eclipse-jdt/eclipse.jdt.ui/blob/master/org.eclipse.jdt.astview.feature/feature.xml#L19

I have also no idea which update site and how to publish?

@akurtakov
Copy link
Contributor

As this is published to jdt web space this is smth that JDT devs handled and not the releng team so I can't help with that. IMHO it makes perfect sense to have it in the regular p2 repo if that is the intention and I can help with that.

@iloveeclipse
Copy link
Member Author

IMHO it makes perfect sense to have it in the regular p2 repo if that is the intention and I can help with that.

Yes, please. I wish I can have them not only in the debugger :-)

@laeubi
Copy link
Contributor

laeubi commented Jan 15, 2023

Why not simply add the bundles that provide them to the jdt.feature instead of an extra one? m2eclise for example also ship some debugging views that are not enabled by default, but if could get interesting.

@mickaelistria
Copy link
Contributor

+1 to add it in the SDK (as a feature or even directly as bundles since IIRC mixed products are now supported).
-1 to add it in the main jdt.feature; as this would be source of noise and confusion for the vast majority of JDT users.

@laeubi
Copy link
Contributor

laeubi commented Jan 15, 2023

-1 to add it in the main jdt.feature; as this would be source of noise and confusion for the vast majority of JDT users.

Can you explain how "the vast majority of JDT users" are confused by a view that is not shown?

@mickaelistria
Copy link
Contributor

Can you explain how "the vast majority of JDT users" are confused by a view that is not shown?

Users will get to the "Show View" dialog and see one more view which they would hardly know whether it's interesting to them or not. Usually, they'll want the Outline view, but if they're not familiar with the Outline term, they may choose the AST view hoping it's what they want, and often, it wouldn't be what they want.
Every new UI entry, every new term does adds complexity to users, because it forces them to chose or to learn 1 more thing.

@laeubi
Copy link
Contributor

laeubi commented Jan 15, 2023

Users will get to the "Show View" dialog and see one more view which they would hardly know whether it's interesting to them or not.

There is already a filter of "default" shown items and a "more..." that actually contains lot of stuff I hardly know if it is interesting, but thing what, I just don't click items I don't know what they do and e voila I'm not confused ;-)

@mickaelistria
Copy link
Contributor

And what about Ctrl+3? Or other entry points?

I just don't click items I don't know what they do and e voila I'm not confused ;-)

Good for you. But I don't think this can be generalized to everyone.

@iloveeclipse
Copy link
Member Author

+1 to add it in the SDK (as a feature or even directly as bundles since IIRC mixed products are now supported).

Could you give a pointer how one can do that? I would be fine to have the tools in the SDK, because this is what SDK is good for. Having them in feature is irrelevant for the purpose I have.

@laeubi
Copy link
Contributor

laeubi commented Jan 15, 2023

Well given that we better ship nothing to the user as then a user can't be "confused" ...

@mickaelistria
Copy link
Contributor

Could you give a pointer how one can do that? I would be fine to have the tools in the SDK, because this is what SDK is good for. Having them in feature is irrelevant for the purpose I have.

You can see for example https://github.com/eclipse-tycho/tycho/blob/master/tycho-its/projects/product.types/products/mixed.product/mixed.product and apply similar addition <plugins> to https://github.com/eclipse-platform/eclipse.platform.releng.aggregator/blob/master/eclipse.platform.releng.tychoeclipsebuilder/eclipse.platform.repository/sdk.product . But actually, it would make sense to add it into the SDK feature https://github.com/eclipse-platform/eclipse.platform.releng/blob/master/features/org.eclipse.sdk/feature.xml directly.

@merks
Copy link
Contributor

merks commented Jan 16, 2023

There's a distinction between what's show here:

image

Versus what's available via Other... Wouldn't it be fine for it to only be available view Other?

@iloveeclipse
Copy link
Member Author

@merks : if it is possible to add the bundles to SDK feature, I will just do that. I simply had no time yet to do that.

@merks
Copy link
Contributor

merks commented Jan 16, 2023

I just ask, because EPP projects uses JDT's feature so that makes a difference where you adding..

@akurtakov
Copy link
Contributor

Exactly, the goal should be to add to eclipse sdk feature and not to the jdt feature

@noopur2507
Copy link
Member

See https://www.eclipse.org/jdt/ui/index.php - Optional Plug-ins (maintained by JDT UI).

Update site mentioned is - http://www.eclipse.org/jdt/ui/update-site/content

We also have these views available via Eclipse Marketplace.

@iloveeclipse
Copy link
Member Author

Update site mentioned is - http://www.eclipse.org/jdt/ui/update-site/content

That was built last time 2021.
image

See https://www.eclipse.org/jdt/ui/index.php - Optional Plug-ins (maintained by JDT UI).

Well, we see how good that maintenance is :-(

The #390 re-enabled building the views that for some reason was not done anymore...

Note: the point is to simplify JDT development to provide all the tools we wrote for the JDT developer instantly.
And I fully agree that they shouldn't be included in "default" Eclipse based packages.
But SDK is the place where I expect them to appear, because they belong to "software development kit" for Eclipse.

@noopur2507
Copy link
Member

Regarding the last update, it was done with #259. Not sure if something went wrong and the changes are not being reflected.

@iloveeclipse
Copy link
Member Author

Regarding the last update, it was done with #259. Not sure if something went wrong and the changes are not being reflected.

So do you propose to keep the current state and rely on manual tasks to update not widely known update site?

What speaks against automatic deployment in the SDK (not part of JDT feature)?

  • It will be automated, zero effort to deployment
  • It will be available instantly in SDK also for those who has no idea about existence of the tools and update site

@mickaelistria
Copy link
Contributor

As far as I understand, @noopur2507 did only share some context and didn't oppose to the idea of adding the views to the SDK feature.
I agree with @iloveeclipse in the current state, adding those to SDK feature would provide good value and make maintenance easier; maybe making it possible for JDT to abandon maintenance of http://www.eclipse.org/jdt/ui/update-site/content and the Marketplace entry: less maintenance, more value delivered to the right audience, isn't it great?

@ktatavarthi
Copy link
Member

+1 for adding these plugins to the sdk.

The approach followed previously was that the ASTView and JEView were updated post Eclipse SDK Release which officially supported a new Java release. I had done it as part of #259.

The problem that @iloveeclipse has discovered seems to have occurred because of the move of the git repository from https://git.eclipse.org/r/plugins/gitiles/www.eclipse.org/jdt to https://github.com/eclipse-jdt/jdt-website. The new changes done as part of #259 are missing here.

@merks
Copy link
Contributor

merks commented Jan 17, 2023

When these are available, I think it will be good that the Oomph setup for JDT installs these views, right? I can make the requirements optional greedy so that that install doesn't fail if it doesn't find those views...

@iloveeclipse
Copy link
Member Author

When these are available, I think it will be good that the Oomph setup for JDT installs these views, right?

Exact. That's was the point of having all the tooling we have at hand.

@merks
Copy link
Contributor

merks commented Jan 17, 2023

I looked at what's contributed to SimRel and because product that includes the SDK feature that now includes these two view features is contributed, I expect that the train repo will also contain these two additional JDT view features:

image

@noopur2507
Copy link
Member

As far as I understand, @noopur2507 did only share some context and didn't oppose to the idea of adding the views to the SDK feature.

That's correct.

@akurtakov
Copy link
Contributor

AST and Java element views are available and installed by default by the SDK in https://download.eclipse.org/eclipse/downloads/drops4/I20230117-0430/ thus this one can be closed.

@iloveeclipse
Copy link
Member Author

Thanks Alex, but I plan to add smoke tests.

@akurtakov
Copy link
Contributor

Perfect. I am just reporting releng part is done. The views will need some polishing for sure but this is beyond this issue purpose.

@merks
Copy link
Contributor

merks commented Jan 17, 2023

FYI, these new things are cause JDT's Maven publishing to fail because of missing sources:

https://ci.eclipse.org/releng/view/Publish%20to%20Maven/job/PublishJDTtoMaven/

The sources are present in the p2 repo, so I shall investigate...

@merks
Copy link
Contributor

merks commented Jan 17, 2023

@akurtakov

The problem is that the source bundles are not mentioned anywhere, and there is no source feature generated for these two things that were added to the SDK feature. I'm pretty sure if we had the corresponding source features to the SDK that this will fix the Maven publishing problem and is the easiest way to fix it...

iloveeclipse added a commit to iloveeclipse/eclipse.jdt.ui that referenced this issue Jan 17, 2023
iloveeclipse added a commit to iloveeclipse/eclipse.jdt.ui that referenced this issue Jan 17, 2023
iloveeclipse added a commit to iloveeclipse/eclipse.jdt.ui that referenced this issue Jan 17, 2023
iloveeclipse added a commit that referenced this issue Jan 17, 2023
@iloveeclipse
Copy link
Member Author

FYI, these new things are cause JDT's Maven publishing to fail because of missing sources:

https://ci.eclipse.org/releng/view/Publish%20to%20Maven/job/PublishJDTtoMaven/

The sources are present in the p2 repo, so I shall investigate...

I believe this continues now in eclipse-platform/eclipse.platform.releng.aggregator#739, so nothing to do here at JDT.

@iloveeclipse
Copy link
Member Author

Unfortunately the smoke test didn't work as expected on Mac & Linux Java 19, I have to update the test => reopening.

begin 0, end -1, length 2

java.lang.StringIndexOutOfBoundsException: begin 0, end -1, length 2
at java.base/java.lang.String.checkBoundsBeginEnd(String.java:4601)
at java.base/java.lang.String.substring(String.java:2704)
at org.eclipse.jdt.ui.tests.views.SmokeViewsTest.readJdkRelease(SmokeViewsTest.java:186)
at org.eclipse.jdt.ui.tests.views.SmokeViewsTest.createProject(SmokeViewsTest.java:151)
at org.eclipse.jdt.ui.tests.views.SmokeViewsTest.setUp(SmokeViewsTest.java:67)

@iloveeclipse iloveeclipse reopened this Jan 18, 2023
iloveeclipse added a commit to iloveeclipse/eclipse.jdt.ui that referenced this issue Jan 18, 2023
Original code was doing too much, we don't need that here since we run
the test in same JVM.

System property "java.specification.version" has all we need

Fixes eclipse-jdt#387
iloveeclipse added a commit that referenced this issue Jan 18, 2023
Original code was doing too much, we don't need that here since we run
the test in same JVM.

System property "java.specification.version" has all we need

Fixes #387
iloveeclipse added a commit to iloveeclipse/www.eclipse.org-eclipse-news that referenced this issue Jan 25, 2023
iloveeclipse added a commit to eclipse-platform/www.eclipse.org-eclipse-news that referenced this issue Jan 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
8 participants