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

Chore: Separate repl into separate project, fix REPL docs output #277

Merged
merged 5 commits into from
Nov 6, 2024

Conversation

jmcardon
Copy link
Member

@jmcardon jmcardon commented Nov 4, 2024

This PR separates the REPL into its own sub-library which helpful for fixing several issues:

  • This improves the compile times of pact-5 for the sake of chainweb integration.
  • It allows us to pull in pandoc, which fixes in our docs issues where our docs in the repl look like the raw markdown docs.

Before this PR:
image

After this PR:
image

PR checklist:

  • Test coverage for the proposed changes
  • PR description contains example output from repl interaction or a snippet from unit test output
  • (If Relevant) Documentation has been (manually) updated at https://docs.kadena.io/pact

Additionally, please justify why you should or should not do the following:

  • Benchmark regressions
  • N/A.
  • Confirm replay/back compat (Ignore until core release)
  • (For Kadena engineers) Run integration-tests against a Chainweb built with this version of Pact (Ignore until core release)

@jmcardon
Copy link
Member Author

jmcardon commented Nov 4, 2024

As of my last commit, it also fixes an annoying issue in the pact-5 repl that repl functions were actually going through the gas charging method, which when it actually charges gas, will essentially brick the repl.

The following is now possible:

pact>(env-gasmodel "table")
"Set gas model to table-based cost model"
pact>(env-gaslimit 1)
"Set gas limit to 1"
pact>(env-gas
env-gas             env-gaslimit        env-gaslog          env-gasmodel        env-gasmodel-fixed
pact>(+ "hello" "world")
(interactive):1:0: Gas limit "1"exceeded: 2
 1 | (+ "hello" "world")
   | ^^^^^^^^^^^^^^^^^^^


pact>(env-gas 0)
"Set gas to 0"

- Move tracing flags to repl sublibrary
- Fix repl builtins charging gas on the repl
@@ -18,7 +18,7 @@ Use the following arguments to specify the test expression and error message for

| Argument | Type | Description |
|----------|--------|------------------------------------------------|
| `expression` | bool | Specifies the expression to evaluate. |
| expression | bool | Specifies the expression to evaluate. |
Copy link
Member

Choose a reason for hiding this comment

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

@lsgunnlsgunn last PR's added the backticks. What is the correct style now, see: https://github.com/kadena-io/pact-5/pull/278/files

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't actually understands why this breaks docusaurus, but it looks weird in our ansi terminal renderer, but lisa can add them back if necessary

Copy link
Contributor

Choose a reason for hiding this comment

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

We don't need backticks around ordinary words or words to indicate syntax, per se. We only require them for certain characters that freak Docusaurus out where the docs engine doesn't know how to interpret things like < and > or { and } except as HTML. I don't know what all the characters are that it doesn't like. Amir put something together that records them, I think.

@jmcardon jmcardon merged commit 5650ae1 into master Nov 6, 2024
13 checks passed
@jmcardon jmcardon deleted the jose/separate-repl branch November 6, 2024 17:12
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.

4 participants