-
Notifications
You must be signed in to change notification settings - Fork 1
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 delayed parsing function to use in flex-tasks
parsing
#252
Conversation
Changed that .md file because the Xrefchecker complained about a redirecting link and failed. |
c335704
to
95a2162
Compare
Ich denke, die Funktion sollte im Sinne von fmidue/flex-tasks#72 (comment) vereinfacht werden. (inlining von Und dann stellt sich noch die Frage, ob die Funktion hier in |
dd4b10d
to
f91ac41
Compare
src/Formula/Parsing/Delayed.hs
Outdated
(\answer -> messaging (either Just (const Nothing) $ | ||
parseDelayedRaw (fully fallBackParser) answer) | ||
) | ||
. delayed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see why this function has Delayed
in its name, given that none of its arguments does actually involve any Delayed
business.
And also its implementation has no reason to use anything like delayed
or parseDelayed
etc. It can simply be written like this:
newFunctionName p messaging fallBackParser =
parseWithOrReport
(parse (fully p) "(answer string)")
(messaging . either Just (const Nothing) . parse (fully fallBackParser) "(answer string)")
I'm actually uncertain whether the fully
calls are really necessary, and whethere ""
instead of "(answer string)"
would make sense.
But the main point is: This function isn't about Delayed
parsing at all. And also it does not really belong in the logic-tasks
repo. It isn't used in logic-tasks
and it also isn't specifically about parsing formulas or other logic concepts. It is just a certain way of doing parsing with a fallback for better error messages.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(After this function is moved to flex-tasks
, it is possible that parseDelayedWithAndThen
below can be expressed in terms of it. That is, importing newFunctionName
from flex-tasks
instead of importing parseWithOrReport
from there. But that is really a different aspect.)
This pull request has now essentially become a closer for #217 instead of what it started out as. |
(Might be better to close it and make a clean new pull request addressing #217.) |
To execute only once and receive either an error message or the result, then use this value for both syntax and semantics checks.