links
links copied to clipboard
Moved dependency chasing to the loader and fixed some tests
This PR:
- removes some
ignorelines in the tests file for tests that are now passing; - fixes some tests;
- moves the dependency chasing to the loader (as the comment suggests) so that we can search for files next to the current one (i.e. while compiling
example/foo.links, the moduleBarwill be searched inexample/bar.linksinstead ofbar.links); - adds the
case otherwise ->construct as an alias to thecase _ ->construct; - adds a message to the testing output in case a test that was skipped is now passing.
I don't know the context of this patch. What's the motivation? It seems to bundle a bunch of unrelated things together. Specifically, what's the motivation for the case change?
- adds the
case otherwise ->construct as an alias to thecase _ ->construct;
There are three tests that use this construct (examples/toggle.links, examples/handlers/toggle.links, and examples/handlers/toggle_web.links). As a side note, otherwise is tokenized to the OTHERWISE token, which only appears once in the entire parser in a location unrelated to this, so the change is safe.
The motivation of the entire PR is to make sure the tests are up-to-date, i.e. that tests which should pass do actually pass, that tests that pass are not ignored, and to fix minor issues (the case otherwise).
The motivation of the entire PR is to make sure the tests are up-to-date, i.e. that tests which should pass do actually pass, that tests that pass are not ignored, and to fix minor issues (the case otherwise).
I think Daniel's point is that unlike fixing tests, this is a language change, and deserves to be pulled out and discussed/agreed sepatrately, as it does not appear to be an already recognized issue that no one had gotten around to yet.
I'm not sure what is wrong with case _, or what is less wrong with case otherwise. Could you please factor this out and make a case for it?
Got it, I have removed this change.
(Also, when the test was created in 1b72bf9d80fa57fb3712e6917c7f712d4734678d, the OTHERWISE token did not exist, but it did when the typechecking test script was added in 4d6687e076ab35cdae6d91dfb1a65e10076d041e.)
The motivation of the entire PR is to make sure the tests are up-to-date, i.e. that tests which should pass do actually pass, that tests that pass are not ignored, and to fix minor issues (the case otherwise).
I think Daniel's point is that unlike fixing tests, this is a language change, and deserves to be pulled out and discussed/agreed sepatrately
Yes indeed. I think the problem is that otherwise became a keyword, when @SimonJF implemented the session exception handling syntax. We have discussed this "issue" previously. I believe the right solution is to unify our various handling-syntaxes into switch (...) { ... } (issue #57 discusses some thoughts on a better internal representation). We currently have special syntax for effect patterns. We'd need special syntax for catching session exceptions too.
Ah - sorry @rajdakin, I misinterpreted your change about case otherwise as meaning that you had introduced this keyword and rather than that you were adjusting things to make ordinary case _ a desugared form of case using otherwise. It does still sound like this is something that should be discussed before further changes, though.
The issue is that session exception syntax is of the form try exp as pattern otherwise exp - it wasn't ever being used as a synonym for _ before. So even if this is a non breaking change, it is a nontrivial one. I feel it is conventional enough to write case _ or similar in functional languages, rather than have a separate keyword for catch-all or default cases; but if we do want to have such a separate keyword, for me it would be more idiomatic to write default => or (to avoid proliferation of keywords) otherwise => rather than case otherwise =>, but this does feel like bikeshedding in action, now.
Is the current version good to go then? I have removed everything concerning case otherwise.