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

[23] JEP 467: Markdown Documentation Comments #2429

Closed
2 of 3 tasks
Tracked by #2212
mpalat opened this issue May 9, 2024 · 21 comments
Closed
2 of 3 tasks
Tracked by #2212

[23] JEP 467: Markdown Documentation Comments #2429

mpalat opened this issue May 9, 2024 · 21 comments
Assignees
Milestone

Comments

@mpalat
Copy link
Contributor

mpalat commented May 9, 2024

ref: https://openjdk.org/jeps/467

Summary
Enable JavaDoc documentation comments to be written in Markdown rather than solely in a mixture of HTML and JavaDoc @-tags.

@mpalat
Copy link
Contributor Author

mpalat commented Jun 3, 2024

@akurtakov
Copy link
Contributor

akurtakov commented Jun 3, 2024

There are both markdown and commonmark (for the subtle differences) implementations available in Eclipse Mylyn project (https://github.com/eclipse-mylyn/org.eclipse.mylyn/tree/main/mylyn.docs/wikitext/core) .
This implementaiton is already used by LSP4E project so it might even be time to move a bundle or two to Eclipse Platform if it becomes requirement for both LSP4E and JDT.
FYI @ruspl-afed

@jarthana
Copy link
Member

jarthana commented Jun 3, 2024

Another point: If we were to use an existing bundle for implementation, we might have to move some of the code (for e.g. JavadocParser) from org.eclipse.jdt.core.compiler.batch to org.eclipse.jdt.core.

@ruspl-afed
Copy link
Contributor

Thank you for referencing me @akurtakov

Regarding move of Wikitext from Eclipse Mylyn to Eclipse Platform:
It does not look impossible, but it needs some third-party libraries that I would not welcome, like Guava.

@stephan-herrmann
Copy link
Contributor

Another point: If we were to use an existing bundle for implementation, we might have to move some of the code (for e.g. JavadocParser) from org.eclipse.jdt.core.compiler.batch to org.eclipse.jdt.core.

I don't think any code in compiler.batch parses HTML, does it? It should be the same for markdown, then.

AFAICS all the interpretation of markup/down happens in jdt.ui, only. Am I missing anything?

@iloveeclipse
Copy link
Member

AFAICS all the interpretation of markup/down happens in jdt.ui, only.

Nope. I recently had to fix org.eclipse.jdt.internal.core.JavadocContents and I believe there are few other places around that parse javadoc.

@jarthana
Copy link
Member

jarthana commented Jun 3, 2024

Another point: If we were to use an existing bundle for implementation, we might have to move some of the code (for e.g. JavadocParser) from org.eclipse.jdt.core.compiler.batch to org.eclipse.jdt.core.

I don't think any code in compiler.batch parses HTML, does it? It should be the same for markdown, then.

AFAICS all the interpretation of markup/down happens in jdt.ui, only. Am I missing anything?

JavadocParser does quite a bit of work for this, for e.g. the Javadoc hover and view. There's some in jdt.ui, which are for partitions and such.

@stephan-herrmann
Copy link
Contributor

So I stand corrected, jdt.core does a bit of HTML parsing.

But is it really done in the compiler?

  • org.eclipse.jdt.internal.core.JavadocContents is jdt.core/model not compiler.batch
  • in JavadocParser I only see parsing of javadoc tags. Where should I look to see HTML parsing?

@iloveeclipse
Copy link
Member

I'm not sure when JavadocParser is called but looking at https://openjdk.org/jeps/467 it seems it is not only html to markdown is the problem but "classic" javadoc comment style /** */ is proposed to be replaced with /// - and that sounds like we need changes in batch compiler. Also links to symbols (types, methods, fields) are supposed to change to something like /// - [a field][String#CASE_INSENSITIVE_ORDER] which will most likely need some kind of compiler change.

@stephan-herrmann
Copy link
Contributor

I'm not sure when JavadocParser is called but looking at https://openjdk.org/jeps/467 it seems it is not only html to markdown is the problem but "classic" javadoc comment style /** */ is proposed to be replaced with /// - and that sounds like we need changes in batch compiler. Also links to symbols (types, methods, fields) are supposed to change to something like /// - [a field][String#CASE_INSENSITIVE_ORDER] which will most likely need some kind of compiler change.

Correct, but still the compiler should not need to "understand" markdown.
Also I assume interpreting the new link format will happen where HTML-anchors are handled now: in JavadocContents (i.e., model).

This would imply

  • o.e.j.c.compiler.batch needs to learn a new style of comments
  • for o.e.jdt.core we have two options:
    • manually parse the new link style, or
    • delegate to some markdown library for this task
  • full interpretation of markdown is only needed in o.e.jdt.ui.

In terms of architecture, my guess is still that only jdt.ui needs a new dependency.

@stephan-herrmann
Copy link
Contributor

Also links to symbols (types, methods, fields) are supposed to change to something like /// - [a field][String#CASE_INSENSITIVE_ORDER] which will most likely need some kind of compiler change.

This, btw, is a bit tricky, because the link target may contain array types denoted with [], which requires some escaping to avoid confusion with the enclosing link syntax. This makes me think that hand coded parsing is most appropriate for this part.

@stephan-herrmann
Copy link
Contributor

Regarding move of Wikitext from Eclipse Mylyn to Eclipse Platform: It does not look impossible, but it needs some third-party libraries that I would not welcome, like Guava.

@ruspl-afed thanks for mentioning Guava, which is notorious for causing version conflicts with, e.g., Xtext. Do you know if guava types are used in any API of wikitext? If so, then it will probably re-export guava and that would lock clients of wikitext to use that specific version of guava.

If not mentioned in any API, we might get away without any conflicts? (as multiple versions my coexist within equinox).

OTOH, how deeply is that dependency built into wikitext? I could imagine that since Java 8 its use is no longer super-relevant? If we'd lend a hand, could it perhaps be refactored to avoid guava?

@stephan-herrmann
Copy link
Contributor

Since this issue affects several layers of our architecture, I'm afraid it could hit us badly when addressed too late.

Could people please weigh in whether the following break-down is appropriate:

  • o.e.j.c.compiler.batch needs to learn a new style of comments
  • for o.e.jdt.core we have two options:
    • manually parse the new link style, or
    • delegate to some markdown library for this task
  • full interpretation of markdown is only needed in o.e.jdt.ui.

In terms of architecture, my guess is still that only jdt.ui needs a new dependency.

@akurtakov akurtakov assigned jarthana and unassigned jarthana Jun 20, 2024
@ruspl-afed
Copy link
Contributor

@stephan-herrmann

Do you know if guava types are used in any API of wikitext?

I do not see them as a part of API signatures, however we have Guava usages in API types

If not mentioned in any API, we might get away without any conflicts? (as multiple versions my coexist within equinox).

Theoretically yes.

OTOH, how deeply is that dependency built into wikitext? I could imagine that since Java 8 its use is no longer super-relevant? If we'd lend a hand, could it perhaps be refactored to avoid guava?

Some types are relatively easy to replace, others may require some effort (special collections, string manipulations). In Mylyn we have a ticket to remove Guava, but I cannot promise any specific delivery date, since we only have a few part-time volunteers for the whole Mylyn codebase.

Moreover, I can see other dependencies involved:

  • javax.lang
  • javax.xml
  • org.jsoup
  • org.w3c.dom
  • org.xml.sax

and a bit custom way of obtaining language support service instance

@iloveeclipse
Copy link
Member

In the past I've seen Guava breaking API very fast, I personally wouldn't like to have it in platform, even if multiple versions are possible, it would be not fun to deal with resulting problems if someone just wants "some" guava package which would available in different versions and of course all incompatible to each other.

@jarthana
Copy link
Member

OK, so the general consensus seems to be doing the hard-parsing like we are doing in JavadocParser with the help of Scanner and let the jdt.ui use third party libraries for the UI part? I will create an issue for UI separately as that seems like the bigger ask.

@stephan-herrmann
Copy link
Contributor

OK, so the general consensus seems to be doing the hard-parsing like we are doing in JavadocParser with the help of Scanner and let the jdt.ui use third party libraries for the UI part? I will create an issue for UI separately as that seems like the bigger ask.

See eclipse-jdt/eclipse.jdt.ui#1472 :)

@stephan-herrmann
Copy link
Contributor

stephan-herrmann commented Jul 25, 2024

Comment moved to #2738 (comment)

@stephan-herrmann
Copy link
Contributor

Based on existing changes in jdt.core, eclipse-jdt/eclipse.jdt.ui#1472 successfully integrates markdown comments in Javadoc view and hovers. See the issue for screenshots :)

@stephan-herrmann
Copy link
Contributor

We'll have a few follow-up issues in JDT/UI, but for a first milestone I declare this issue completed.

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

No branches or pull requests

6 participants