[WIP] refactor(cli): Librarify nova_cli to avoid code duplication
#916
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
niluxv
changed the title
nova_cli to avoid code duplicationnova_cli to avoid code duplication
niluxv
force-pushed
the
librarify-cli
branch
from
262a806 to
5bcfde9
Compare
This already factored out some logic that was duplicated between the `eval` and `repl` subcommands. However, the real goal is to provide a library the benchmark runner can use, to avoid code duplication there.
niluxv
force-pushed
the
librarify-cli
branch
from
5bcfde9 to
0dbdc3a
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Right now this is mostly moving code around, so the benchmark runner (which I have in a separate branch) can share most of its code with the cli. I have no idea whether this library could be useful for users of nova as well, I don't really know their requirements, but I assume the current setup is not flexible enough for that. (For example, currently everything assumes there is only one realm.)
The benchmark runner needs to separately parse and execute scripts, while the cli can just immediately execute a parsed script, which is why I haven't touched the script parsing and execution logic of the cli. However, maybe we can store a list of parsed but not yet executed scripts in
HostDefined(alongside the module map of already executed modules, or maybe merge them?), and then implement convenience commands for script/module parsing and execution for theInstance. Or maybe that is overly complicated and it is better to leave this work to the library user.I would love to hear any suggestions, as I find myself really struggling to provide a flexible and convenient API.