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

[Oomph-Setup] Add jdt.debug configuration setup #533

Merged
merged 1 commit into from
Oct 11, 2024

Conversation

HannesWell
Copy link
Contributor

What it does

Add jdt.debug configuration setup.
Additionally add a styled and drag&drop-able Oomph Configuration button to the main README.

Part of eclipse-platform/eclipse.platform.releng.aggregator#2430

Author checklist

Additionally add a styled and drag&drop-able Oomph Configuration button
to the main README.

Part of eclipse-platform/eclipse.platform.releng.aggregator#2430
@HannesWell
Copy link
Contributor Author

@jukzi or @iloveeclipse, could you please submit this as well? Thanks in advance.

<description>The JDT Debug workspace provides all the source code of the project.</description>
</workspace>
<description>
&lt;p>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is "<" encoded with lt but ">" is not gt encoded?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Oomph-Editor is doing this. I assume it's not necessary.
It's the same for other configurations as well.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also in exiting configurations i can not read such fields:
image
it's illegal xml. @merks can you fix that?
I'll be AFK 2w

@merks
Copy link
Contributor

merks commented Oct 11, 2024

The only problem I see is that these PRs need to be merged:

#533
eclipse-jdt/eclipse.jdt.ui#1710
eclipse-jdt/eclipse.jdt.core#3071

That's the reason for these three broken references:

image

Could you please merge those too?


It's normal that only up to the first newline character is shown in the properties view. The cell editor shows the whole text:

image

The hover also shows it nicely formatted:

image

As does this page:

https://www.eclipse.org/setups/installer/?url=https://raw.githubusercontent.com/eclipse-jdt/eclipse.jdt/master/org.eclipse.jdt.releng/JDTConfiguration.setup&show=true

So no, a > is not invalid XML in element content. That's why EMF does not escape them for element content:

https://github.com/eclipse-emf/org.eclipse.emf/blob/e810c433fd597d68a246cb384251214a301e871e/plugins/org.eclipse.emf.ecore.xmi/src/org/eclipse/emf/ecore/xmi/impl/XMLSaveImpl.java#L3339-L3375

But does for attribute content:

https://github.com/eclipse-emf/org.eclipse.emf/blob/e810c433fd597d68a246cb384251214a301e871e/plugins/org.eclipse.emf.ecore.xmi/src/org/eclipse/emf/ecore/xmi/impl/XMLSaveImpl.java#L3496-L3538

@jukzi
Copy link
Contributor

jukzi commented Oct 11, 2024

So no, a > is not invalid XML in element content

The right angle bracket (>) may be represented using the string " &gt; ", and MUST, [for compatibility](https://www.w3.org/TR/xml/#dt-compat), be escaped using either " &gt; " or a character reference when it appears in the string " ]]> " in content, when that string is not marking the end of a [CDATA section](https://www.w3.org/TR/xml/#dt-cdsection).
https://www.w3.org/TR/xml/

ok, in this case it is not invalid. while in general it may be invalid. So i would always encode it to be on the valid side, but that doesn't matter for this pr.

@jukzi jukzi merged commit 6cab2be into eclipse-jdt:master Oct 11, 2024
11 checks passed
@HannesWell HannesWell deleted the oomph-configuration branch October 11, 2024 17:47
@merks
Copy link
Contributor

merks commented Oct 11, 2024

ok, in this case it is not invalid. while in general it may be invalid. So i would always encode it to be on the valid side, but that doesn't matter for this pr.

In general it's never invalid in element content. Moreover, EMF has done it this way for 25 years without anyone ever suggesting it should be changed. If it ain't broke, don't fix it. 😬

@HannesWell
Copy link
Contributor Author

Thank you Jörg for submitting this!

The results look good and as expected:

grafik

And if you click it, the installer page looks good too:
https://www.eclipse.org/setups/installer/?url=https://raw.githubusercontent.com/eclipse-jdt/eclipse.jdt.debug/master/org.eclipse.jdt.debug.setup/JdtDebugConfiguration.setup&show=true

@merks
Copy link
Contributor

merks commented Oct 11, 2024

Cool. 😎

@merks
Copy link
Contributor

merks commented Oct 12, 2024

@jukzi

FYI, the corner cases are handled properly by EMF:

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants