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

Add a simple extended error message (#4646) #5568

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Add a simple extended error message (#4646) #5568

wants to merge 6 commits into from

Conversation

christianbender
Copy link

I fixed the issue #4646.
Now we get for the code the following error message

void setup() {
  size(200, 200, FX2D);
}

void draw() {
  boolean c = (boolean) 1 || (boolean) 1;
}

produces the error message
screenshot

instead of mere 23)

I put in a chunk of code that check the size of the error message. If the size to small (mere 3 characters) then the error message will be extended.

@christianbender christianbender changed the title simple extended error message (#4646) Add a simple extended error message (#4646) Jul 1, 2018
@gohai
Copy link
Contributor

gohai commented Jul 3, 2018

That's clearly better than what it said before - but just curious: could this be made to return just "int constant cannot be casted into boolean", in a general way that would not have to be a special-case?

@christianbender
Copy link
Author

@gohai
I can catch the case 23) and then print int constant cannot be casted into boolean. for this case.
The advantage of my solution is that we can catch such error messages like 23) complete with a extended message-text.

@christianbender
Copy link
Author

christianbender commented Jul 3, 2018

@gohai I changed the mechanism. Now we can catch right away the case 23). In other cases the error message will be extended. such as the example above. The advantage: if we found another case (like 23)) we can simply replenish the code structure for the mechanism.
screenshot

@gohai
Copy link
Contributor

gohai commented Jul 4, 2018

You might want to remove the debug println() calls again in your commit. Also the <= 3 seems a bit arbitrary - thinking that it might make sense to compare the full line of an error that behaves as expected, with the line that you get for int constant cannot be casted into boolean, and modify the errorFormat regular expression accordingly. Guessing that the regex is simply not taking every possible output into account.

Otherwise, as a band-aid, your current patch is clearly already an improvement (without the printlns). Thanks!

@christianbender
Copy link
Author

christianbender commented Jul 4, 2018

@gohai

  • I removed my println(...) statments.
  • I reworte the mechanism, using regex. Checks whether the error message only comprise of words.

Thanks for your feedback.

@gohai
Copy link
Contributor

gohai commented Jul 4, 2018

Hi @christianbender - I see you created another regex. My suggest was rather to modify this one so that it would work with whatever is going on with 23) also.

If you look here you see that your pull requests still has some random changes that come with it, like commented-out println() statements. My git tricks, in the hope that they are useful for you also: I stage changes with git add -p, where I am being asked block by block whether I want to commit this particular change to the next commit [y] or not [n].

When I learn that I made a mistake in a previous commit I fix with git rebase -i HEAD~1. This lets you mark the commits you want to edit by placing an "e" at the beginning of each line. Now you can make retroactive changes to this commit, add them to be committed, and commit with git commit --amend, which picks up the commit message you already wrote. (This also works with more then just the recent commit, by changing HEAD~1 to HEAD~2 and so on.) And lastly, you can also use git rebase to "squash" (merge) different commits into one. This works by placing an "s" of the beginning of the line for the commits that you want to merge with its predecessor (also works with multiple). This way you can have one, logical commit at the end of a process of trial and error - instead of a spew of commits that do and undo various things. Note: whenever you rewrite history (this is what git rebase does), you need to push your changes to GitHub using git push -f - by default GitHub doesn't like history to be rewritten (it's indeed not a good practice, except for pull-requests branches, so before they are in a tree that other people might depend on).

@christianbender
Copy link
Author

christianbender commented Jul 4, 2018

@gohai

  • I changed the regex. And test it.
  • I removed useless println statments in the code.

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.

None yet

2 participants