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(gnovm): Add Basic Code Indentation in REPL #1596

Open
wants to merge 16 commits into
base: master
Choose a base branch
from

Conversation

notJoon
Copy link
Member

@notJoon notJoon commented Jan 29, 2024

Description

Indent for code

Updated the code in the REPL to adjust the indent level when an indent rule is detected.

I was considering enhancing the situation where closing parentheses feel tightly attached to the beginning of the previous line. In this case, it seems like setting the terminal to raw mode and directly recognizing the user's key inputs to adjust the level would suffice. (maybe later?)

Example

$ go run ./gnovm/cmd/gno repl

gno> type Foo struct {
...     a int
...     b int
...     }
type Foo struct {
        a       int
        b       int
}

gno> func main() {
...     val := Foo {
...         a: 10,
...         b: 20,
...         }
...     println(val.a + val.b)
...     }
func main() {
        val := Foo{
                a:      10,
                b:      20,
        }
        println(val.a + val.b)
}

gno> main()
30

Add Clear Command

Added the /clear command to clear the repl's screen. For testing, I used dependency injection.

@github-actions github-actions bot added the 📦 🤖 gnovm Issues or PRs gnovm related label Jan 29, 2024
Copy link

codecov bot commented Jan 29, 2024

Codecov Report

Attention: Patch coverage is 47.52475% with 53 lines in your changes missing coverage. Please review.

Project coverage is 58.21%. Comparing base (e7e47d2) to head (15e10d2).

Files Patch % Lines
gnovm/cmd/gno/repl.go 48.48% 48 Missing and 3 partials ⚠️
gnovm/pkg/repl/repl.go 0.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1596      +/-   ##
==========================================
+ Coverage   54.62%   58.21%   +3.58%     
==========================================
  Files         581      389     -192     
  Lines       77955    65077   -12878     
==========================================
- Hits        42584    37884    -4700     
+ Misses      32194    24377    -7817     
+ Partials     3177     2816     -361     
Flag Coverage Δ
contribs/gnodev ?
contribs/gnofaucet ?
contribs/gnokeykc ?
contribs/gnomd ?
gno.land ?
gnovm 60.05% <47.52%> (+0.02%) ⬆️
misc/autocounterd ?
misc/genproto ?
misc/genstd ?
misc/goscan ?
misc/logos ?
misc/loop ?
tm2 ?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@notJoon notJoon changed the title [WIP] REPL update feat(gnovm): REPL update Feb 7, 2024
@notJoon notJoon marked this pull request as ready for review February 7, 2024 02:48
Copy link
Contributor

@deelawn deelawn left a comment

Choose a reason for hiding this comment

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

Thanks Joon. There are some cases here where the indentation feature exhibits strange behavior when indentation-causing characters are part of string literals rather than the code. Have any ideas how to solve this?

gnovm/cmd/gno/repl.go Outdated Show resolved Hide resolved
gnovm/cmd/gno/repl.go Outdated Show resolved Hide resolved
gnovm/cmd/gno/repl.go Outdated Show resolved Hide resolved
gnovm/cmd/gno/repl.go Outdated Show resolved Hide resolved
gnovm/cmd/gno/repl.go Outdated Show resolved Hide resolved
gnovm/cmd/gno/repl_test.go Show resolved Hide resolved
@notJoon notJoon requested a review from a team as a code owner April 4, 2024 05:55
@notJoon notJoon marked this pull request as draft April 4, 2024 10:19
@notJoon notJoon marked this pull request as ready for review April 5, 2024 07:38
@notJoon notJoon requested a review from deelawn April 5, 2024 07:38
@notJoon
Copy link
Member Author

notJoon commented Apr 5, 2024

Memo: Once this PR is merged, I plan to implement the cursor movement and history save/load features proposed in issue #1416. Additionally, to achieve more sophisticated indentation control, I intend to process while keeping the terminal in raw mode, as it requires the ability to read user inputs directly.

@notJoon notJoon changed the title feat(gnovm): REPL update feat(gnovm): Add Basic Code Indentation in REPL Apr 5, 2024
@@ -108,6 +108,11 @@ type Repl struct {
stdin io.Reader
}

// Read implements io.Reader.
func (r *Repl) Read(p []byte) (n int, err error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this necessary? Things seem to work okay without it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh, i added that line to test the raw terminal mode. i forget to remove this.

gnovm/cmd/gno/repl.go Outdated Show resolved Hide resolved
gnovm/cmd/gno/repl.go Outdated Show resolved Hide resolved
gnovm/cmd/gno/repl_test.go Outdated Show resolved Hide resolved
gnovm/cmd/gno/repl_test.go Outdated Show resolved Hide resolved
trimmedLine := strings.TrimSpace(line)

indentLevel = updateIndentLevel(trimmedLine, indentLevel)
line, inEdit = handleEditor(line)
Copy link
Contributor

Choose a reason for hiding this comment

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

This change breaks the editor mode. I'd recommend restoring it since it is only called from this one location and moving it to its own function doesn't have too much benefit. The issue can by entering editor mode with /editor and then doing some variable assignments. Notice that editor mode is exited after doing a single variable assignment when it should remain in editor mode until the semicolon is encountered.

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

@deelawn deelawn left a comment

Choose a reason for hiding this comment

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

Thanks for the updates. It looks a lot better 👍 . Please see the comments I left. A few things seem to be broken and a couple others unnecessary. The remaining comments are just suggestions.

@notJoon
Copy link
Member Author

notJoon commented Apr 11, 2024

Thanks for the updates. It looks a lot better 👍 . Please see the comments I left. A few things seem to be broken and a couple others unnecessary. The remaining comments are just suggestions.

Thank you for the detailed review again. I'll try to get it done as soon as possible this week. 👍👍

notJoon and others added 2 commits April 11, 2024 16:28
@zivkovicmilos
Copy link
Member

Hey @notJoon, what's the status on this PR?

Can you please merge in the latest master changes?

@notJoon notJoon requested a review from ajnavarro as a code owner June 14, 2024 12:44
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.

I'm a bit skeptical about this pull request.

The code is redundant with parsing capabilities present elsewhere to analyze the source, and I'm not convinced of its usefulness: if you enter a block with the REPL, it is already reformatted and displayed correctly, even if not indented correctly, as following:

gno> func d() string { return "a" +
...  "b" + "x" }
func d() string {
	return "a" +
		"b" + "x"
}

gno>

@moul
Copy link
Member

moul commented Jun 29, 2024

What about using ast/parser then ast/format?

@notJoon
Copy link
Member Author

notJoon commented Jul 1, 2024

What about using ast/parser then ast/format?

That would be practical. I'll ping you after update current code. 👍

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
Status: In Progress
Status: In Progress
Status: No status
Status: In Review
Development

Successfully merging this pull request may close these issues.

5 participants