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

Fix #825 - Showing wrong errors' line numbers for .java files #937

Merged
merged 1 commit into from
Jan 28, 2025

Conversation

e1dem
Copy link
Contributor

@e1dem e1dem commented Jan 27, 2025

Using PreprocSketch.mapJavaToSketch result for processing .java files is incorrect, as a sketch's combined source code file does not include code from .java tabs.

As far as I can see, there is a similar issue with showing variables usage, so I would suggest this solution as a temporary one.

Closes #825 (originally at benfry/processing4#825)

Copy link
Collaborator

@Stefterv Stefterv left a comment

Choose a reason for hiding this comment

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

Hi @e1dem! Thank you for your contribution, code looks good, testing now on macOS

@Stefterv
Copy link
Collaborator

Screenshot 2025-01-28 at 10 12 54 Looking good!

@Stefterv Stefterv requested a review from SableRaf January 28, 2025 09:13
@Stefterv
Copy link
Collaborator

@SableRaf Please take a look and merge if you understand what has changed!

@SableRaf
Copy link
Collaborator

@Stefterv: So, if I understand correctly, the changes ensure that we don't need the workaround to zero index for PDE tabs because Java tabs live in their own files, and the line number offset is now handled correctly. Did I get that right?

And does the fix still work when you have more than one Java tab?

@Stefterv
Copy link
Collaborator

Yes it works with more than one Java tab

Not sure what workaround you are referring to... But yes the line number offset is now handled correctly

@SableRaf
Copy link
Collaborator

Hmm then I'm not sure I understand 100%. If this is a low stakes change then it may be ok but if you need a stricter review, we might need another pair of eyes on this.

@Stefterv Stefterv merged commit e53ff13 into processing:main Jan 28, 2025
6 checks passed
@Stefterv
Copy link
Collaborator

This is a low-stakes quality of life change
Basically it add an check in the convertIProblem() function that in case of a Java file gets the correct line numbering

@SableRaf
Copy link
Collaborator

@all-contributors please add @e1dem for code

Copy link
Contributor

@SableRaf

I've put up a pull request to add @e1dem! 🎉

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.

Wrong line numbers in error messages when using .java files
3 participants