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 toml support for pyk options #4254

Merged
merged 33 commits into from
Apr 24, 2024
Merged

Conversation

Baltoli
Copy link
Contributor

@Baltoli Baltoli commented Apr 16, 2024

@Baltoli Baltoli added the pyk Issues transferred from runtimeverification/pyk label Apr 16, 2024
@Baltoli Baltoli assigned Baltoli and ovatman and unassigned Baltoli Apr 16, 2024
@Baltoli
Copy link
Contributor Author

Baltoli commented Apr 18, 2024

@ovatman The K repository requires that changes are based on the develop branch rather than master; if you merge master into your PR (5fbfaf2) then you'll end up with this versioning noise as well as your actual changes.

@ovatman ovatman marked this pull request as ready for review April 18, 2024 10:39
@ovatman ovatman requested review from nwatson22 and removed request for dwightguth, rv-jenkins, ehildenb and F-WRunTime April 18, 2024 10:39
@ehildenb
Copy link
Member

@ovatman you'll need to rebase on develop branch directly and remove all the introduce intermediate commits then force push.

@ovatman
Copy link
Contributor

ovatman commented Apr 19, 2024

@ovatman you'll need to rebase on develop branch directly and remove all the introduce intermediate commits then force push.

@ehildenb
I did the rebasing yesterday but I didn't force push, I think that's why it didn't seem to work.

Copy link
Member

@nwatson22 nwatson22 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me, once it has been rebased properly and the KEVM and kontrol branches are working. I like the solution of having the mapping from command-line string format to python variable format.

There may be a way to avoid having to mention all the superclasses in from_option_string, by adding a new function in Options here similar to the constructor that collects and merges the from_option_strings for any type of Options.

There is a lot of boilerplate that has to be duplicated across all 3 repos, but this was also the case with version of Options that was merged. Hopefully eventually we will be wrapping the argparse and toml parsing logic and consolidating most of the boilerplate into pyk only (Similar to what is mentioned in #4188)

geo2a and others added 7 commits April 22, 2024 13:00
Fixes #4197

This PR allows calling `pyk prove` with a custom server executable via
`pyk prove --kore-rpc-command COMMAND`, which is especially useful when
giving logging options to the server, i.e.:
```
pyk prove test-spec.k --verbose --kore-rpc-command 'kore-rpc-booster -l Simplify'
```
fixes #4187 

The pyk outer lexer now has location information for attribute tokens.

The unit tests for the lexer now check that the token text actually
appears at its location in the source text.

---------

Co-authored-by: rv-jenkins <[email protected]>
@rv-jenkins rv-jenkins merged commit 25d3a4a into develop Apr 24, 2024
17 checks passed
@rv-jenkins rv-jenkins deleted the tolga/toml-options-rebased branch April 24, 2024 18:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge pyk Issues transferred from runtimeverification/pyk
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants