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 delayed parsing function to use in flex-tasks parsing #252

Closed
wants to merge 10 commits into from

Conversation

patritzenfeld
Copy link
Member

To execute only once and receive either an error message or the result, then use this value for both syntax and semantics checks.

@patritzenfeld
Copy link
Member Author

Changed that .md file because the Xrefchecker complained about a redirecting link and failed.

@patritzenfeld patritzenfeld marked this pull request as ready for review February 4, 2025 11:10
@patritzenfeld patritzenfeld marked this pull request as draft February 4, 2025 12:48
@jvoigtlaender
Copy link
Member

Ich denke, die Funktion sollte im Sinne von fmidue/flex-tasks#72 (comment) vereinfacht werden. (inlining von delayed etc.)

Und dann stellt sich noch die Frage, ob die Funktion hier in logic-tasks bleiben sollte oder in fmidue/flex-tasks#77 integriert. Wenn Delayed nicht mehr im Spiel ist, könnte das ja durchaus als eine Funktion angesehen werden, die allgemein in flex-tasks Konfigurationen nützlich ist, nicht auf Logik-Aufgaben beschränkt.

@patritzenfeld patritzenfeld marked this pull request as ready for review February 5, 2025 15:15
(\answer -> messaging (either Just (const Nothing) $
parseDelayedRaw (fully fallBackParser) answer)
)
. delayed
Copy link
Member

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.

Copy link
Member

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.)

@patritzenfeld patritzenfeld marked this pull request as draft February 6, 2025 10:22
@jvoigtlaender
Copy link
Member

This pull request has now essentially become a closer for #217 instead of what it started out as.

@jvoigtlaender
Copy link
Member

(Might be better to close it and make a clean new pull request addressing #217.)

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.

2 participants