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 parse error handler for precompute args failures #93

Merged
merged 4 commits into from Aug 25, 2016

Conversation

Projects
None yet
3 participants
@kroepke
Member

kroepke commented Aug 19, 2016

allow lowercase timezone ids

fixes #84

add parse error handler for precompute args failures
allow lowercase timezone ids

@kroepke kroepke added this to the 1.1.0 milestone Aug 19, 2016

paramPosition++;
}
return "Unable to pre-compute value for " + paramPosition + ". argument " + failure.getArgumentName() + " in call to function " + function.descriptor().name() + ": " + failure.getCause().getMessage();

This comment has been minimized.

@hc4

hc4 Aug 19, 2016

There is should be "," before argument,right?

This comment has been minimized.

@kroepke

kroepke Aug 19, 2016

Member

Actually there should be "st", "nd", "rd", "th" there, but I was too lazy for an error message ;)

It's the argument position, since we don't highlight it yet.

This comment has been minimized.

@hc4

hc4 Aug 19, 2016

Maybe write:
return "Unable to pre-compute value for argument " + failure.getArgumentName() + " (pos. "+paramPosition +") in call..

This comment has been minimized.

@kroepke

kroepke Aug 19, 2016

Member

Personally I like the abbreviated ordinal better, so I got unlazy and implemented that. The error now reads:
Unable to pre-compute value for 1st argument timezone in call to function now_in_tz: The datetime zone id '123' is not recognised

This comment has been minimized.

@hc4

hc4 Aug 19, 2016

Cool 👍
Maybe put argument name in quotes for better readability?

This comment has been minimized.

@kroepke

kroepke Aug 19, 2016

Member

I think for now I'll skip that, because then I would need to touch all the other errors, too.
We also don't know how we will continue with the editor in 2.2 so I'd like to leave it at this.

This comment has been minimized.

@hc4

hc4 Aug 19, 2016

Ok.
Anyway 3rd much better than 3. :)

kroepke added some commits Aug 19, 2016

@edmundoa edmundoa self-assigned this Aug 25, 2016

@edmundoa

This comment has been minimized.

Member

edmundoa commented Aug 25, 2016

LGTM 👍

@edmundoa edmundoa merged commit 5a9bfda into master Aug 25, 2016

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@edmundoa edmundoa deleted the issue-84 branch Aug 25, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment