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 more info to names() in Python API #1512

Merged
merged 8 commits into from
Apr 20, 2023
Merged

Add more info to names() in Python API #1512

merged 8 commits into from
Apr 20, 2023

Conversation

m-yac
Copy link
Contributor

@m-yac m-yac commented Apr 13, 2023

As touched upon in #1492, currently the remote API does not provide as much information about names as the browse command does in the REPL. This PR expands the result of the visible names method (i.e. names() in the Python API) to include:

  • all the additional information provided by the browse command (i.e. what module the name is from, whether or not it is a parameter, and whether or it not it is a property), as well as
  • fixity information (i.e. whether or not the name is an infix operator, and if so its associativity and precedence level).

This PR also adds the property_names and parameter_names functions to the Python API, which return only those names which are properties or parameters, respectively.

Finally, this PR also fixes a bug where names() would sometimes hang when a parameterized module is loaded.

See test_names.py for an example of this new output of names as well as the new the property_names and parameter_names functions.

Looking ahead: At some point it might also be nice to have type_names and module_names commands which return similar results to names, just for these other two distinct namespaces in Cryptol. It might also be nice to have an easy way to prove a property retrieved from property_names.

Copy link
Contributor

@RyanGlScott RyanGlScott 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 reasonable to me, aside from one design question.

cryptol-remote-api/src/CryptolServer/Names.hs Outdated Show resolved Hide resolved
@m-yac m-yac requested review from yav and removed request for eddywestbrook April 13, 2023 20:20
cryptol-remote-api/CHANGELOG.md Outdated Show resolved Hide resolved
cryptol-remote-api/python/CHANGELOG.md Outdated Show resolved Hide resolved
@m-yac m-yac force-pushed the better_names_cmd branch from 6b2154f to 76f1db6 Compare April 13, 2023 22:30
@m-yac
Copy link
Contributor Author

m-yac commented Apr 20, 2023

This PR should be ready now. This last commit gives types to the output of names, property_names, parameter_names, and focused_module in the Python API as additional documentation for what sort of values these functions return (see the newly added CryptolNameInfo and CryptolModuleInfo in cryptoltypes.py).

Note that in making that change, I converted the type fields of the output of names() to CryptolSchemas for the first time, and found a bug where for unknown constructors, the arguments field wasn't getting converted to JSON.

@m-yac m-yac merged commit d5a4463 into master Apr 20, 2023
@weaversa
Copy link
Collaborator

I realize I'm months late, but would it be easy to add the line number where the definition of the name appears to the info that's returned?

@RyanGlScott RyanGlScott deleted the better_names_cmd branch March 22, 2024 14:50
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