Skip to content

SketchException refactor #1164

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

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

joshgiesbrecht
Copy link

Created an :app:utils module, moved SketchException to a processing.utils package within :app:utils, changed all references to SketchException in both java and app to use processing.utils.SketchException, removed the two duplicate SketchException classes in app and java.preproc.

This currently seems to work (tested only on Linux so far, but I don't think there's anything platform-specific involved), and does solve the bug of highlighting the first error on running a sketch. Non-Java modes will not yet be using the correct SketchException class; I suspect this will be a 'soft' fail that has a similar problem to the existing bug, but I haven't tested this much yet. (Shader mode seems to be broken already?)

Since the draft PR for #1104 also creates :app:utils and is more complex than this PR, I'm fine with waiting until the other PR is merged, and then I can make sure this one merges cleanly before it's added in.

What's the right thing to do re: the impact on non-Java modes? Should I submit PRs for those projects as well, maybe once this one's actually merged into main?

fixes bug with selecting an error that's in a tab that isn't visible yet.
…ls.SketchException, except I'm missing something because on compile it's not finding it. (module / package visibility problem or somethin')
@joshgiesbrecht
Copy link
Author

uhhhh just noticed this PR is re-including commits for the previous bug fix I did. is that a problem, other than being kind of ugly?

@joshgiesbrecht
Copy link
Author

(Also, I can do some more testing yet, but wanted to check that the merge timing made sense and to ask about the other modes)

((also I like parentheticals too much and am probably overthinking what to say, ha ha))

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.

1 participant