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

Exposed parse method in TLAPM API #147

Merged
merged 7 commits into from
Sep 11, 2024
Merged

Conversation

ahelwer
Copy link
Collaborator

@ahelwer ahelwer commented Aug 15, 2024

This adds a simple string -> parse tree method in TLAPM's public API, so it can be hit by non-inline unit tests.

@kape1395
Copy link
Collaborator

I added a similar function in the LSP branch.

https://github.com/kape1395/tlapm/blob/0ac8eac06ccb96a4cb138e622e15fa4aaccf41ac/src/tlapm_lib.ml#L617

Mine does more about related modules. Is is meaningful to keep them both?

@ahelwer
Copy link
Collaborator Author

ahelwer commented Aug 28, 2024

I think so, because it would be nice to just generate a syntax tree without doing anything else. For example, I would want to check how EXTENDS and INSTANCE statements are parsed without having to actually have those modules in existence.

@kape1395
Copy link
Collaborator

@ahelwer, the LSP branch has now been merged.
Can you fix the conflict? What do you think about renaming

  • my function: module_of_string -> elaborate_module_of_string
  • and your: parse_module_from_string -> parse_module_of_string?

@ahelwer
Copy link
Collaborator Author

ahelwer commented Aug 28, 2024

How about transitive_parse_module_of_string for your method?

@kape1395
Copy link
Collaborator

I sent you a PR with an attempt to resolve this naming clash.

@kape1395
Copy link
Collaborator

Why have you reverted modctx_of_string to transitive_parse_module_of_string. I think the latter is not very precise.

src/tlapm_lib.ml Outdated Show resolved Hide resolved
@kape1395 kape1395 merged commit d8ade9a into tlaplus:main Sep 11, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants