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

Code after Javadocs still folded "too much" #1992

Open
danthe1st opened this issue Jan 31, 2025 · 4 comments · May be fixed by #1993
Open

Code after Javadocs still folded "too much" #1992

danthe1st opened this issue Jan 31, 2025 · 4 comments · May be fixed by #1993

Comments

@danthe1st
Copy link

danthe1st commented Jan 31, 2025

While testing #1825, I noticed the following issue:

In #1562, @jakub-suliga introduced the fix for #1939 (see this comment) but there seems to be a case where this issue still happens.

With the following code, folding a() causes b() to be invisible as well in the current master (after merging #1562) if the new/experimental folding is disabled:

package org.example.test;

class X {
	/*
	 * a b
	 */
	void a() {
	} void b() {
	}
}
@danthe1st
Copy link
Author

danthe1st commented Jan 31, 2025

Also, it seems like that PR changes the test in #1960 to only consider the "new"/experimental folding and does not run any tests with the "old" folding (I expected it to run tests with both options).

@jakub-suliga Is this intentional?

@danthe1st
Copy link
Author

Also @jjohnstn since you reviewed the PR in question, I think you should know about that as well.

jakub-suliga added a commit to jakub-suliga/eclipse.jdt.ui that referenced this issue Jan 31, 2025
If the second method starts in the same line the first one ends then
there will still be 2 foldings.

Fixes eclipse-jdt#1992
jakub-suliga added a commit to jakub-suliga/eclipse.jdt.ui that referenced this issue Jan 31, 2025
If the second method starts in the same line the first one ends then
there will still be 2 foldings.

Fixes eclipse-jdt#1992
@jakub-suliga jakub-suliga linked a pull request Jan 31, 2025 that will close this issue
@jakub-suliga
Copy link
Contributor

Also, it seems like that PR changes the test in #1960 to only consider the "new"/experimental folding and does not run any tests with the "old" folding (I expected it to run tests with both options).

@jakub-suliga Is this intentional?

No, it isnt intentional. I fixed it in #1993

@jakub-suliga
Copy link
Contributor

With the following code, folding a() causes b() to be invisible as well in the current master (after merging #1562) if the new/experimental folding is disabled:

The PR #1993 fixes the porblem too. I accidentally set includeLastLine to true a second time, which caused the issue.

jakub-suliga added a commit to jakub-suliga/eclipse.jdt.ui that referenced this issue Feb 5, 2025
If the second method starts in the same line the first one ends then
there will still be 2 foldings.

Fixes eclipse-jdt#1992
jakub-suliga added a commit to jakub-suliga/eclipse.jdt.ui that referenced this issue Feb 5, 2025
If the second method starts in the same line the first one ends then
there will still be 2 foldings.

Fixes eclipse-jdt#1992
jakub-suliga added a commit to jakub-suliga/eclipse.jdt.ui that referenced this issue Feb 5, 2025
If the second method starts in the same line the first one ends then
there will still be 2 foldings.

Fixes eclipse-jdt#1992
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 a pull request may close this issue.

2 participants