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.

Sign up for GitHub

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

Jump to bottom

[23] Folding a markdown region hides the first line of subsequent Java #2943

Open
wants to merge 2 commits into
base: BETA_JAVA23
Choose a base branch
from
Open

[23] Folding a markdown region hides the first line of subsequent Java #2943

wants to merge 2 commits into from
+7 −4

Conversation

jarthana
Copy link
Member

code

What it does

Fixes eclipse-jdt/eclipse.jdt.ui#1615

How to test

Author checklist

@jarthana jarthana marked this pull request as draft September 11, 2024 11:33
@jarthana
Copy link
Member Author

This fix introduces a problem in a corner case when the markdown ends with a param or other incomplete tag and no whitespace before the closing \n, which is not part of the markdown doc. I have adjusted a testcase to pass, but will take that up via a different issue.

@jarthana jarthana marked this pull request as ready for review September 12, 2024 06:29
@jarthana jarthana force-pushed the gh1615 branch 2 times, most recently from 8a8304f to e8b552b Compare September 16, 2024 06:38
@jarthana
Copy link
Member Author

OK, this is stuck with a hung test case. I don't think this is going in for the GA release. Will take it up post-merge.

@stephan-herrmann
Copy link
Contributor

OK, this is stuck with a hung test case. I don't think this is going in for the GA release. Will take it up post-merge.

time to try again ? 😄

@jarthana
Copy link
Member Author

I recall this being held is related to eclipse-jdt/eclipse.jdt.ui#1848, which was on hold for another reason. But now I see that is green. Will take up both if time permits.

@stephan-herrmann
Copy link
Contributor

Strange things going on here.

I pulled the change and locally merged with BETA_JAVA24. Result:

  • folding works 😄
  • I can't reproduce the test hang
  • rendering to HTML is broken
  • I get 17 fail 1 error in org.eclipse.jdt.ui.tests.hover.MarkdownCommentTests

stephan-herrmann
stephan-herrmann reviewed Feb 28, 2025
View reviewed changes
org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/parser/Scanner.java
@@ -1980,6 +1981,7 @@ protected int getNextToken0() throws InvalidInputException {
}
}
if (!lineBeginsWithMarkdown()) {
this.currentPosition--;
Copy link
Contributor

Choose a reason for hiding this comment

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

Reverting this one line change brings back proper rendering to HTML and fixes org.eclipse.jdt.ui.tests.hover.MarkdownCommentTests

Found this only by experimentation, have no idea what I'm talking about :)

stephan-herrmann added a commit to stephan-herrmann/eclipse.jdt.core that referenced this pull request Feb 28, 2025
@stephan-herrmann
add the test change from PR eclipse-jdt#2943
e2e63e5
@stephan-herrmann
Copy link
Contributor

See #3760

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stephan-herrmann stephan-herrmann stephan-herrmann left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jarthana @stephan-herrmann