Stream: contributing

Topic: Parser Package

Luke Boswell (Jan 16 2023 at 17:44):

I have been trying to write tests and examples for the Parser example. I was hoping to use this to demonstrate basic packages functionality. I managed to get it working ok; but came across the following issue#4904 when using Parser.Core.{ many }. I'm reasonably confident I am doing things correctly there, so there is a bug in the compiler somewhere.

My idea was to write more tests/examples to demonstrate the parsers. Would it make sense to refactor this example to make the Parser into a package instead of having it's own platform? We can then have a number of different examples perhaps using the basic-cli URL package if required? In the future we could even have CI build the Parser package to provide a URL perhaps?

Following is an example header using which uses basic packages functionality. It works when I include a package module declaration as below at roc/examples/parser/main.roc.

app "example"
    packages {
        cli: "https://github.com/roc-lang/basic-cli/releases/download/0.1.2/3bKbbmgtIfOyC6FviJ9o8F8xqKutmXgjCJx3bMfVTSo.tar.br",
        parser: "main.roc",
    }
    imports [
        cli.Stdout,
        parser.Parser.Core.{ Parser, parsePartial, buildPrimitiveParser, many },
    ]
    provides [ main ] to cli

package "parser"
    exposes [
        Parser.Core,
        Parser.CSV,
        Parser.Str,
        Parser.Http,
    ]
    packages {}

Brendan Hansknecht (Jan 16 2023 at 17:48):

I am pretty sure that feature simply isn't implemented yet, but I totally could be wrong. I haven't tested/look at this in a long while. So it very well could have changed.

Luke Boswell (Jan 16 2023 at 17:48):

Note that if we made it a package, we can remove the redundant "Parser" in the imports.

Luke Boswell (Jan 16 2023 at 17:49):

Yeah it works. PR#4736 was what inspired me to start playing with it.

Anton (Jan 16 2023 at 17:49):

I think @Richard Feldman already implemented URL packages but there was some caveat I forgot about.

Brian Carroll (Jan 16 2023 at 17:49):

Yeah that makes loads of sense! The only reason it has its own platform is that, when it was written, that was the only way we could do things. But your proposed structure would be much better. And it's good for the examples to show off things like this.

Brian Carroll (Jan 16 2023 at 17:54):

Luke if you have managed to get this working, I think it would be an excellent contribution to the repo! I'd be happy to help or review, and I'm sure others here would too.

Brian Carroll (Jan 16 2023 at 17:55):

oh wait you said there's a compiler bug

Brian Carroll (Jan 16 2023 at 17:56):

Can you make a PR out of your branch so that people can pull it and try to help fix it?

Brian Carroll (Jan 16 2023 at 18:02):

I use the many combinator in the Http parser for headers and it works OK there

Brian Carroll (Jan 16 2023 at 18:03):

I notice you're using parsePartial rather than making it parse the whole input. So I'm wondering if that doesn't work well in combination with many.

Luke Boswell (Jan 16 2023 at 18:09):

Sure thing!

I created a draft PR#4906 here with what I have so far. I'm heading out for dinner now, but happy to continue plugging away at it tomorrow.

Brian Carroll (Jan 16 2023 at 18:13):

OK so many calls manyImpl which is recursive, and the recursion terminates when the parser fails. But your parser never fails. So that could be the issue.

Brian Carroll (Jan 16 2023 at 18:13):

I suppose all parsers should fail when the input is exhausted though. But maybe that's not implemented correctly in the library.

Brian Carroll (Jan 16 2023 at 18:14):

Anyway I'm not convinced this is a compiler issue, I think it's more likely a library issue.

Brian Carroll (Jan 16 2023 at 18:15):

yes that's it. manyImpl handles all errors the same in a branch with Err _ ->. But it should have a special case for the error where we ran out of characters to parse!

Brian Carroll (Jan 16 2023 at 18:15):

..I think

Richard Feldman (Jan 16 2023 at 21:21):

the only caveat I know about with packages right now is that I haven't yet implemented packages which depend on other packages - there's some more work that needs to be done to support that use case

Richard Feldman (Jan 16 2023 at 21:22):

it might also apply to platforms which depend on packages - I forget

Richard Feldman (Jan 16 2023 at 21:22):

but either way, applications depending directly on packages - URL or otherwise - should work fine!

Brian Carroll (Jan 17 2023 at 08:23):

@Luke Boswell , there's actually an infinite loop in your program!
The many combinator keeps recursing until the inner parser fails. But your letterParser does not explicitly handle the case where input is empty, so it actually succeeds, and returns Ok { val: Other, input: [] }.
This means the inner parser never fails. It just keeps returning Other forever, so the many recursion keeps going, and the program hangs.

Luke Boswell (Jan 17 2023 at 09:00):

Thank you, I'll keep working on these examples/tests and see if I can propose a sensible refactor based on similar libraries. @Anton does it make sense to have tests that can or should be run in CI or sometime? I'm not sure of the best way to do that. I was thinking of writing example apps and then writing expects in them, which we can run roc test on maybe.

Anton (Jan 17 2023 at 09:06):

@Anton does it make sense to have tests that can or should be run in CI or sometime? I'm not sure of the best way to do that. I was thinking of writing example apps and then writing expects in them, which we can run roc test on maybe.

Yeah, that sounds great @Luke Boswell. We can use a similar setup like with the basic-cli repo. I'd say we probably also want a separate repo for this parser package.

Brian Carroll (Jan 17 2023 at 13:44):

We have a set of Rust tests that test every example app . Recently I added a new variant that can run roc check on a library module like Parser.Http too.

Brian Carroll (Jan 17 2023 at 13:46):

But if we're making it external then maybe it's better to follow how basic-cli does it

Last updated: Jul 05 2025 at 12:14 UTC