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

feat(repl): support multi-line inputs and declarations #978

Merged
merged 13 commits into from
Aug 10, 2023

Conversation

ajnavarro
Copy link
Contributor

@ajnavarro ajnavarro commented Jul 17, 2023

gno-repl

How to run it manually:

  • go to gnovm folder
  • run make install
  • execute gno repl

BREAKING CHANGE: repl inputs now are different.

Related issues:

Refactor repl to make it more useful:

  • Added reset command to clean the state
  • No need to specify if you are adding a function, import, interface, struct or an expression
  • Functions support
  • Constants support Will be done on a following PR
  • Struct and Interfaces support Will be done on a following PR
  • Variables support Will be done on a following PR
  • Unit tests
  • Imports support
  • Repl multiline input support

Contributors Checklist

  • Added new tests, or not needed, or not feasible
  • Provided an example (e.g. screenshot) to aid review or the PR is self-explanatory
  • Updated the official documentation or not needed
  • No breaking changes were made, or a BREAKING CHANGE: xxx message was included in the description
  • Added references to related issues and PRs
  • Provided any useful hints for running manual tests
  • Added new benchmarks to generated graphs, if any. More info here.

@github-actions github-actions bot added the 📦 🤖 gnovm Issues or PRs gnovm related label Jul 17, 2023
@ajnavarro ajnavarro force-pushed the improvement/repl branch 3 times, most recently from 5b8e146 to 79b8800 Compare July 20, 2023 10:26
gnovm/tests/imports.go Outdated Show resolved Hide resolved
@moul
Copy link
Member

moul commented Jul 20, 2023

Looks promising!

Note that I intend to reuse the new Repl{} object outside of the CLI and will need to pass custom Stdin/Stdout. Specifically, I plan to experiment with ttys.

If you're confident, I recommend creating a gnovm/pkg/repl package now to make it more generic. Otherwise, we can migrate it later.

@ajnavarro
Copy link
Contributor Author

If you're confident, I recommend creating a gnovm/pkg/repl package now to make it more generic. Otherwise, we can migrate it later.

@moul all the repl logic is already included into gnovm/pkg/repl with new tests.

@ajnavarro ajnavarro marked this pull request as ready for review July 25, 2023 09:57
@ajnavarro ajnavarro requested a review from a team as a code owner July 25, 2023 09:57
@zivkovicmilos zivkovicmilos self-requested a review July 26, 2023 14:45
Copy link
Member

@zivkovicmilos zivkovicmilos left a comment

Choose a reason for hiding this comment

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

Looks and works great 💯

I've left some super-minor comments.

Love the gif demo, we should standardize it for PRs

gnovm/cmd/gno/repl.go Outdated Show resolved Hide resolved
gnovm/cmd/gno/repl.go Show resolved Hide resolved
gnovm/pkg/repl/repl.go Outdated Show resolved Hide resolved
gnovm/pkg/repl/repl.go Outdated Show resolved Hide resolved
gnovm/pkg/repl/repl_test.go Show resolved Hide resolved
gnovm/pkg/repl/repl_test.go Show resolved Hide resolved
gnovm/tests/imports.go Show resolved Hide resolved
gnovm/pkg/repl/repl.go Outdated Show resolved Hide resolved
gnovm/pkg/repl/repl.go Outdated Show resolved Hide resolved
Copy link
Member

@gfanton gfanton left a comment

Choose a reason for hiding this comment

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

LGTM 👍 (just one minor detail, see comment)

Copy link
Contributor

@mvertes mvertes left a comment

Choose a reason for hiding this comment

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

Mostly cosmetic suggestions. The code looks good to me otherwise.

gnovm/pkg/repl/repl_test.go Show resolved Hide resolved
gnovm/pkg/repl/repl.go Outdated Show resolved Hide resolved
gnovm/pkg/repl/repl.go Outdated Show resolved Hide resolved
gnovm/pkg/repl/repl.go Outdated Show resolved Hide resolved
gnovm/pkg/repl/repl.go Outdated Show resolved Hide resolved
gnovm/pkg/repl/repl.go Outdated Show resolved Hide resolved
gnovm/pkg/repl/repl.go Outdated Show resolved Hide resolved
gnovm/pkg/repl/repl.go Outdated Show resolved Hide resolved
gnovm/pkg/repl/repl.go Outdated Show resolved Hide resolved
gnovm/pkg/repl/repl.go Outdated Show resolved Hide resolved
gnovm/cmd/gno/repl.go Outdated Show resolved Hide resolved
gnovm/pkg/repl/repl.go Outdated Show resolved Hide resolved
Copy link
Contributor

@harry-hov harry-hov left a comment

Choose a reason for hiding this comment

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

Some minor nitpicking. Overall LGTM 💯

@moul
Copy link
Member

moul commented Aug 10, 2023

🎉 Congratulations on having the most approved PR! 🎉

Signed-off-by: Antonio Navarro Perez <antnavper@gmail.com>
Signed-off-by: Antonio Navarro Perez <antnavper@gmail.com>
Signed-off-by: Antonio Navarro Perez <antnavper@gmail.com>
Signed-off-by: Antonio Navarro Perez <antnavper@gmail.com>
Signed-off-by: Antonio Navarro Perez <antnavper@gmail.com>
Signed-off-by: Antonio Navarro Perez <antnavper@gmail.com>
Signed-off-by: Antonio Navarro Perez <antnavper@gmail.com>
Signed-off-by: Antonio Navarro Perez <antnavper@gmail.com>
Signed-off-by: Antonio Navarro Perez <antnavper@gmail.com>
Signed-off-by: Antonio Navarro Perez <antnavper@gmail.com>
Signed-off-by: Antonio Navarro Perez <antnavper@gmail.com>
Signed-off-by: Antonio Navarro Perez <antnavper@gmail.com>
@github-actions github-actions bot added 🧾 package/realm Tag used for new Realms or Packages. 📦 🌐 tendermint v2 Issues or PRs tm2 related labels Aug 10, 2023
@github-actions github-actions bot removed 🧾 package/realm Tag used for new Realms or Packages. 📦 🌐 tendermint v2 Issues or PRs tm2 related labels Aug 10, 2023
@ajnavarro ajnavarro merged commit 0beb958 into gnolang:master Aug 10, 2023
47 checks passed
@ajnavarro ajnavarro deleted the improvement/repl branch August 10, 2023 10:08
Doozers pushed a commit to Doozers/gno that referenced this pull request Aug 31, 2023
@moul moul added this to the 🌟 main.gno.land (wanted) milestone Sep 8, 2023
mvertes added a commit to mvertes/gno that referenced this pull request Sep 15, 2023
This is a followup of gnolang#978. Instead of starting in multi-line mode prior
to submit, the line is parsed, and new inputs are appended to it as long
as the statment is not complete, as detected by the Go scanner.

This is simpler and more general than previous attempt. The secondary
prompt is "...", different from primary "gno>", similarly to many REPL
programs (node, python, bash, ...).

The "/editor" command is removed as not useful anymore. Note also
that it is now possible to exit using Ctrl-D.
mvertes added a commit to mvertes/gno that referenced this pull request Oct 19, 2023
This is a followup of gnolang#978. Instead of starting in multi-line mode prior
to submit, the line is parsed, and new inputs are appended to it as long
as the statment is not complete, as detected by the Go scanner.

This is simpler and more general than previous attempt. The secondary
prompt is "...", different from primary "gno>", similarly to
many REPL programs (node, python, bash, ...).

The "/editor" command is removed as not useful anymore. Note also
that it is now possible to exit using Ctrl-D.
thehowl pushed a commit that referenced this pull request Nov 7, 2023
![demo](https://github.com/gnolang/gno/assets/5792239/308e61bc-bdf9-498b-9fa7-cd756835f774)

This is a followup of #978. Instead of starting in multi-line mode prior
to submit, the line is parsed, and new inputs are appended to it as long
as the statment is not complete, as detected by the Go scanner.

This is simpler and more general than previous attempt. The secondary
prompt is "...", different from primary "gno>", similarly to many REPL
programs (node, python, bash, ...).

The "/editor" command is removed as not useful anymore. Note also that
it is now possible to exit using Ctrl-D.

Related issues: #446 #950

<!-- please provide a detailed description of the changes made in this
pull request. -->

<details><summary>Contributors' checklist...</summary>

- [x] Added new tests, or not needed, or not feasible
- [x] Provided an example (e.g. screenshot) to aid review or the PR is
self-explanatory
- [x] Updated the official documentation or not needed
- [x] No breaking changes were made, or a `BREAKING CHANGE: xxx` message
was included in the description
- [x] Added references to related issues and PRs
- [ ] Provided any useful hints for running manual tests
- [ ] Added new benchmarks to [generated
graphs](https://gnoland.github.io/benchmarks), if any. More info
[here](https://github.com/gnolang/gno/blob/master/.benchmarks/README.md).
</details>

---------

Co-authored-by: Manfred Touron <94029+moul@users.noreply.github.com>
gfanton pushed a commit to gfanton/gno that referenced this pull request Nov 9, 2023
![demo](https://github.com/gnolang/gno/assets/5792239/308e61bc-bdf9-498b-9fa7-cd756835f774)

This is a followup of gnolang#978. Instead of starting in multi-line mode prior
to submit, the line is parsed, and new inputs are appended to it as long
as the statment is not complete, as detected by the Go scanner.

This is simpler and more general than previous attempt. The secondary
prompt is "...", different from primary "gno>", similarly to many REPL
programs (node, python, bash, ...).

The "/editor" command is removed as not useful anymore. Note also that
it is now possible to exit using Ctrl-D.

Related issues: gnolang#446 gnolang#950

<!-- please provide a detailed description of the changes made in this
pull request. -->

<details><summary>Contributors' checklist...</summary>

- [x] Added new tests, or not needed, or not feasible
- [x] Provided an example (e.g. screenshot) to aid review or the PR is
self-explanatory
- [x] Updated the official documentation or not needed
- [x] No breaking changes were made, or a `BREAKING CHANGE: xxx` message
was included in the description
- [x] Added references to related issues and PRs
- [ ] Provided any useful hints for running manual tests
- [ ] Added new benchmarks to [generated
graphs](https://gnoland.github.io/benchmarks), if any. More info
[here](https://github.com/gnolang/gno/blob/master/.benchmarks/README.md).
</details>

---------

Co-authored-by: Manfred Touron <94029+moul@users.noreply.github.com>
moul added a commit to moul/gno that referenced this pull request Nov 14, 2023
![demo](https://github.com/gnolang/gno/assets/5792239/308e61bc-bdf9-498b-9fa7-cd756835f774)

This is a followup of gnolang#978. Instead of starting in multi-line mode prior
to submit, the line is parsed, and new inputs are appended to it as long
as the statment is not complete, as detected by the Go scanner.

This is simpler and more general than previous attempt. The secondary
prompt is "...", different from primary "gno>", similarly to many REPL
programs (node, python, bash, ...).

The "/editor" command is removed as not useful anymore. Note also that
it is now possible to exit using Ctrl-D.

Related issues: gnolang#446 gnolang#950

<!-- please provide a detailed description of the changes made in this
pull request. -->

<details><summary>Contributors' checklist...</summary>

- [x] Added new tests, or not needed, or not feasible
- [x] Provided an example (e.g. screenshot) to aid review or the PR is
self-explanatory
- [x] Updated the official documentation or not needed
- [x] No breaking changes were made, or a `BREAKING CHANGE: xxx` message
was included in the description
- [x] Added references to related issues and PRs
- [ ] Provided any useful hints for running manual tests
- [ ] Added new benchmarks to [generated
graphs](https://gnoland.github.io/benchmarks), if any. More info
[here](https://github.com/gnolang/gno/blob/master/.benchmarks/README.md).
</details>

---------

Co-authored-by: Manfred Touron <94029+moul@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📦 🤖 gnovm Issues or PRs gnovm related
Projects
Archived in project
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

6 participants