-
Notifications
You must be signed in to change notification settings - Fork 588
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: redirect console output #542
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -2,6 +2,7 @@ package otto | |||||||||||||||||
|
||||||||||||||||||
import ( | ||||||||||||||||||
"fmt" | ||||||||||||||||||
"io" | ||||||||||||||||||
"os" | ||||||||||||||||||
"strings" | ||||||||||||||||||
) | ||||||||||||||||||
|
@@ -15,12 +16,24 @@ func formatForConsole(argumentList []Value) string { | |||||||||||||||||
} | ||||||||||||||||||
|
||||||||||||||||||
func builtinConsoleLog(call FunctionCall) Value { | ||||||||||||||||||
fmt.Fprintln(os.Stdout, formatForConsole(call.ArgumentList)) //nolint:errcheck // Nothing we can do if this fails. | ||||||||||||||||||
var w io.Writer | ||||||||||||||||||
if call.runtime.stdoutWriter != nil { | ||||||||||||||||||
w = call.runtime.stdoutWriter | ||||||||||||||||||
} else { | ||||||||||||||||||
w = os.Stdout | ||||||||||||||||||
} | ||||||||||||||||||
fmt.Fprintln(w, formatForConsole(call.ArgumentList)) //nolint:errcheck // Nothing we can do if this fails. | ||||||||||||||||||
return Value{} | ||||||||||||||||||
} | ||||||||||||||||||
|
||||||||||||||||||
func builtinConsoleError(call FunctionCall) Value { | ||||||||||||||||||
fmt.Fprintln(os.Stdout, formatForConsole(call.ArgumentList)) //nolint:errcheck // Nothing we can do if this fails. | ||||||||||||||||||
var w io.Writer | ||||||||||||||||||
if call.runtime.stdoutWriter != nil { | ||||||||||||||||||
w = call.runtime.stdoutWriter | ||||||||||||||||||
} else { | ||||||||||||||||||
w = os.Stdout | ||||||||||||||||||
} | ||||||||||||||||||
fmt.Fprintln(w, formatForConsole(call.ArgumentList)) //nolint:errcheck // Nothing we can do if this fails. | ||||||||||||||||||
Comment on lines
+30
to
+36
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion: simplify as above.
Suggested change
|
||||||||||||||||||
return Value{} | ||||||||||||||||||
} | ||||||||||||||||||
|
||||||||||||||||||
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -222,6 +222,7 @@ package otto | |||||||||||||||||
import ( | ||||||||||||||||||
"encoding/json" | ||||||||||||||||||
"errors" | ||||||||||||||||||
"io" | ||||||||||||||||||
"strings" | ||||||||||||||||||
|
||||||||||||||||||
"github.com/robertkrimen/otto/file" | ||||||||||||||||||
|
@@ -238,12 +239,15 @@ type Otto struct { | |||||||||||||||||
} | ||||||||||||||||||
|
||||||||||||||||||
// New will allocate a new JavaScript runtime. | ||||||||||||||||||
func New() *Otto { | ||||||||||||||||||
func New(opts ...Option) *Otto { | ||||||||||||||||||
o := &Otto{ | ||||||||||||||||||
runtime: newContext(), | ||||||||||||||||||
} | ||||||||||||||||||
o.runtime.otto = o | ||||||||||||||||||
o.runtime.traceLimit = 10 | ||||||||||||||||||
for _, opt := range opts { | ||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion: lets set the default so we can simplify the use after.
Suggested change
|
||||||||||||||||||
opt(o) | ||||||||||||||||||
} | ||||||||||||||||||
if err := o.Set("console", o.runtime.newConsole()); err != nil { | ||||||||||||||||||
panic(err) | ||||||||||||||||||
} | ||||||||||||||||||
|
@@ -775,3 +779,11 @@ func (o Object) MarshalJSON() ([]byte, error) { | |||||||||||||||||
} | ||||||||||||||||||
return json.Marshal(goValue) | ||||||||||||||||||
} | ||||||||||||||||||
|
||||||||||||||||||
type Option func(*Otto) | ||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion: lets document the type. |
||||||||||||||||||
|
||||||||||||||||||
func WithStdoutWriter(w io.Writer) Option { | ||||||||||||||||||
return func(o *Otto) { | ||||||||||||||||||
o.runtime.stdoutWriter = w | ||||||||||||||||||
Comment on lines
+785
to
+787
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion: lets document the type and change the naming so it easier for the user to understand the impact of using this option.
Suggested change
|
||||||||||||||||||
} | ||||||||||||||||||
} |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -1803,6 +1803,35 @@ func TestOttoInterrupt(t *testing.T) { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
func TestOtterStdoutWriter(t *testing.T) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
tests := []struct { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
name string | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
script string | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
expect string | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}{ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
name: "console.log", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
script: "console.log('hello', 'world')", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
expect: "hello world\n", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
name: "console.err", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
script: "console.error('error', 'world')", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
expect: "error world\n", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
for _, tc := range tests { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
t.Run(tc.name, func(t *testing.T) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
var buf = &bytes.Buffer{} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
vm := New(WithStdoutWriter(buf)) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
_, err := vm.Run(tc.script) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
require.NoError(t, err) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
require.Equal(t, tc.expect, buf.String()) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+1806
to
+1833
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion: can we use sub tests vs table driven as its more concise and allows developers to run specific tests directly from their editor e.g. VS Code. Also the use of just a declared buffer vs instantiated is more idiomatic. Not tested the below, so there might be typos, but you get the idea 😃
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
func BenchmarkNew(b *testing.B) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
for i := 0; i < b.N; i++ { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
New() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -5,6 +5,7 @@ import ( | |||||
"encoding/json" | ||||||
"errors" | ||||||
"fmt" | ||||||
"io" | ||||||
"math" | ||||||
"path" | ||||||
"reflect" | ||||||
|
@@ -66,6 +67,7 @@ type runtime struct { | |||||
stackLimit int | ||||||
traceLimit int | ||||||
lck sync.Mutex | ||||||
stdoutWriter io.Writer | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion: make it clear the use vs the value
Suggested change
|
||||||
} | ||||||
|
||||||
func (rt *runtime) enterScope(scop *scope) { | ||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion: with the use of a default we can simply this.