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

Cleanup that adds { } to single blocks mess up with //$NON-NLS-1$ #1951

Closed
laeubi opened this issue Jan 19, 2025 · 2 comments · Fixed by #1989
Closed

Cleanup that adds { } to single blocks mess up with //$NON-NLS-1$ #1951

laeubi opened this issue Jan 19, 2025 · 2 comments · Fixed by #1989
Assignees
Labels
bug Something isn't working
Milestone

Comments

@laeubi
Copy link
Contributor

laeubi commented Jan 19, 2025

I perform the cleanup to add missing { } to single blocks but it seems to be confused by //$NON-NLS-1$ tags (or comments in general?)

One example is this org.eclipse.equinox.internal.p2.core.AgentLocation it produces this:

Image

please note that there is a space added before the tab, but there is an additional line break, it should actually be

Image

Another one is org.eclipse.equinox.internal.p2.core.Activator.substituteVar(String, String, String) but it is even working in the same class with adjustTrailingSlash method but completely mess up computeLocationSharedAgent where then even a new warning is produced because the //$NON-NLS-1$ tags are not on the right line anymore.

@jjohnstn jjohnstn self-assigned this Jan 24, 2025
@jjohnstn jjohnstn added the bug Something isn't working label Jan 24, 2025
@jjohnstn jjohnstn added this to the 4.35 M3 milestone Jan 24, 2025
jjohnstn added a commit to jjohnstn/eclipse.jdt.ui-1 that referenced this issue Jan 29, 2025
@jjohnstn jjohnstn mentioned this issue Jan 29, 2025
3 tasks
jjohnstn added a commit to jjohnstn/eclipse.jdt.ui-1 that referenced this issue Jan 30, 2025
@laeubi
Copy link
Contributor Author

laeubi commented Jan 31, 2025

@jjohnstn mayn thanks for fixing this I'll try it out once a new I-Build is there!

@laeubi
Copy link
Contributor Author

laeubi commented Jan 31, 2025

Looking more closely on the change... does it really fix the problem? It just seems to add a new testacse ... I'm confused.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants