Skip to content

Update to pratter 5 #1196

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

gabrielhdt
Copy link
Collaborator

@gabrielhdt gabrielhdt commented Feb 12, 2025

From pratter 4, it's possible to interpret a single token in different ways. This allows for instance to consider the symbol - as both the unary and binary minus.

@gabrielhdt gabrielhdt mentioned this pull request Feb 12, 2025
2 tasks
@01mf02
Copy link
Contributor

01mf02 commented Feb 14, 2025

Is the bumping of the minimal OCaml version really required here? Perhaps this is now not necessary anymore after our monad omission ...? :)

@gabrielhdt
Copy link
Collaborator Author

I'm afraid the version 4.14 is required by the operation Seq.uncons, monads aren't guilty there

@01mf02
Copy link
Contributor

01mf02 commented Feb 19, 2025

I guess you could then very easily replace Seq.uncons by something equivalent in Pratter. It's currently used only here:

let tok : ('tok, 'tok option) parser =
 fun inp ->
  match Seq.uncons inp with
  | None -> Ok (None, Seq.empty)
  | Some (t, rest) -> Ok (Some t, rest)

Unfolding Seq.uncons, this could easily be something like (modulo importing Nil and Cons):

let tok : ('tok, 'tok option) parser =
 fun inp ->
  match inp () with
  | Nil -> Ok (None, Seq.empty)
  | Cons (t, rest) -> Ok (Some t, rest)

Not tested, but if uncons is the only reason for bumping the MSOV (minimal supported OCaml version), it might be worth to reconsider that. ;)

@fblanqui
Copy link
Member

Hi @gabrielhdt . Michael's suggestion looks nice and easy to implement. Do you want to try it? I will then wait for the version 5.1 of pratter and an update of this PR.

@gabrielhdt
Copy link
Collaborator Author

Looks good to me ! I'll give it a shot.

@gabrielhdt
Copy link
Collaborator Author

FYI I've managed to reduced the minimal supported ocaml version to 4.10.

On the question of whether this version is low enough or not, I tend to think that if a program is compatible with Debian stable, it's ok.

Now I just have to publish all that to opam :)

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.

3 participants