From 96cc1283cdd48a9671bdc183af767ebfdaa6095d Mon Sep 17 00:00:00 2001
From: Mattias Wadman <mattias.wadman@gmail.com>
Date: Mon, 1 Nov 2021 16:50:28 +0100
Subject: [PATCH] interp: Eval options in jq instead of calling jq from go

Simpler and causes less weird performance issues
---
 .vscode/settings.json           |  1 +
 doc/TODO.md                     |  1 +
 pkg/interp/funcs.go             | 10 +++++-----
 pkg/interp/funcs.jq             | 20 ++++++++++++--------
 pkg/interp/grep.jq              | 13 +++++++++++--
 pkg/interp/internal.jq          | 13 +++----------
 pkg/interp/interp.go            | 32 ++++----------------------------
 pkg/interp/options.jq           |  6 ++----
 pkg/interp/repl.jq              | 19 ++++++++++++++++---
 pkg/interp/testdata/args.fqtest |  1 -
 10 files changed, 55 insertions(+), 61 deletions(-)

diff --git a/.vscode/settings.json b/.vscode/settings.json
index f451d0d2..4934e938 100644
--- a/.vscode/settings.json
+++ b/.vscode/settings.json
@@ -33,6 +33,7 @@
         "Errorer",
         "errorln",
         "esds",
+        "eval",
         "Exif",
         "Exiter",
         "FALLID",
diff --git a/doc/TODO.md b/doc/TODO.md
index 0848cc01..6fc838b5 100644
--- a/doc/TODO.md
+++ b/doc/TODO.md
@@ -14,6 +14,7 @@
 - `format/0` overlap with jq builtin `format/1`. What to rename it to? `decode_format`?
 - repl expression returning a value that produced lots of output can't be interrupted. This is becaus ctrl-C currently only interrupts the evaluation of the expression, outputted value is printed (`display`) by parent.
 - Rework cli/repl user interrupt (context cancel via ctrl-c), see comment in Interp.Main
+- Optimize `Interp.Options` calls, now called per display. Cache per eval? needs to handle nested evals.
 
 ### TODO and ideas
 
diff --git a/pkg/interp/funcs.go b/pkg/interp/funcs.go
index f3192d08..912729fb 100644
--- a/pkg/interp/funcs.go
+++ b/pkg/interp/funcs.go
@@ -61,7 +61,7 @@ func (i *Interp) makeFunctions() []Function {
 
 		{[]string{"_tobitsrange"}, 0, 2, i._toBitsRange, nil},
 
-		{[]string{"tovalue"}, 0, 1, i.toValue, nil},
+		{[]string{"_tovalue"}, 1, 1, i._toValue, nil},
 
 		{[]string{"hex"}, 0, 0, makeStringBitBufTransformFn(
 			func(r io.Reader) (io.Reader, error) { return hex.NewDecoder(r), nil },
@@ -664,7 +664,7 @@ func (i *Interp) format(c interface{}, a []interface{}) interface{} {
 }
 
 func (i *Interp) _display(c interface{}, a []interface{}) gojq.Iter {
-	opts := i.OptionsEx(a...)
+	opts := i.Options(a[0])
 
 	switch v := c.(type) {
 	case Display:
@@ -734,9 +734,9 @@ func (i *Interp) _toBitsRange(c interface{}, a []interface{}) interface{} {
 	return bv
 }
 
-func (i *Interp) toValue(c interface{}, a []interface{}) interface{} {
+func (i *Interp) _toValue(c interface{}, a []interface{}) interface{} {
 	v, _ := toValue(
-		func() Options { return i.OptionsEx(append([]interface{}{}, a...)...) },
+		func() Options { return i.Options(a[0]) },
 		c,
 	)
 	return v
@@ -921,7 +921,7 @@ func (i *Interp) _bitsMatch(c interface{}, a []interface{}) gojq.Iter {
 }
 
 func (i *Interp) _hexdump(c interface{}, a []interface{}) gojq.Iter {
-	opts := i.OptionsEx(a...)
+	opts := i.Options(a[0])
 	bv, err := toBufferView(c)
 	if err != nil {
 		return gojq.NewIter(err)
diff --git a/pkg/interp/funcs.jq b/pkg/interp/funcs.jq
index 7145a3d3..292fdd7e 100644
--- a/pkg/interp/funcs.jq
+++ b/pkg/interp/funcs.jq
@@ -54,22 +54,26 @@ def decode($name; $opts):
 def decode($name): decode($name; {});
 def decode: decode(options.decode_format; {});
 
+def tovalue($opts): _tovalue(options($opts));
+def tovalue: _tovalue({});
 
-def display($opts): _display($opts);
-def display: _display({});
-def d($opts): _display($opts);
-def d: _display({});
-def full($opts): _display({array_truncate: 0} + $opts);
+def display($opts): _display(options($opts));
+def display: display({});
+def d($opts): display($opts);
+def d: display({});
+
+def full($opts): display({array_truncate: 0} + $opts);
 # TODO: rename, gets mixed up with f args often
 def full: full({});
 def f($opts): full($opts);
 def f: full;
-def verbose($opts): _display({verbose: true, array_truncate: 0} + $opts);
+def verbose($opts): display({verbose: true, array_truncate: 0} + $opts);
 def verbose: verbose({});
 def v($opts): verbose($opts);
 def v: verbose;
-def hexdump($opts): _hexdump({display_bytes: 0} + $opts);
-def hexdump: _hexdump({display_bytes: 0});
+
+def hexdump($opts): _hexdump(options({display_bytes: 0} + $opts));
+def hexdump: hexdump({display_bytes: 0});
 def hd($opts): hexdump($opts);
 def hd: hexdump;
 
diff --git a/pkg/interp/grep.jq b/pkg/interp/grep.jq
index 090aad63..5a8054d9 100644
--- a/pkg/interp/grep.jq
+++ b/pkg/interp/grep.jq
@@ -1,3 +1,12 @@
+# TODO: remove once symbolic value is done properly
+def _grep_tovalue:
+  ( if _is_decode_value then
+      ( ._symbol as $s
+      | if $s != "" then $s end
+      )
+    end
+  );
+
 def _grep($v; filter_cond; string_cond; other_cond):
   if $v | type == "string" then
     ( ..
@@ -10,14 +19,14 @@ def _grep($v; filter_cond; string_cond; other_cond):
   end;
 
 def _value_grep_string_cond($v; $flags):
-  ( _tovalue
+  ( _grep_tovalue
   | if type == "string" then test($v; $flags)
     else false
     end
   )? // false;
 
 def _value_grep_other_cond($v; $flags):
-  ( _tovalue
+  ( _grep_tovalue
   | . == $v
   )? // false;
 
diff --git a/pkg/interp/internal.jq b/pkg/interp/internal.jq
index 8f858471..f1679557 100644
--- a/pkg/interp/internal.jq
+++ b/pkg/interp/internal.jq
@@ -34,7 +34,9 @@ def _input_decode_errors(f): _global_var("input_decode_errors"; f);
 def _variables: _global_var("variables");
 def _variables(f): _global_var("variables"; f);
 
-# eval f and finally eval fin even on empty or error
+# eval f and finally eval fin even on empty or error.
+# note that if f outputs more than one value fin will be called
+# for each value.
 def _finally(f; fin):
   ( try f // (fin | empty)
     catch (fin as $_ | error)
@@ -100,15 +102,6 @@ def _eval($expr; $filename; f; on_error; on_compile_error):
     else on_error
     end;
 
-# TODO: remove one symbolic value is done properly?
-def _tovalue:
-  ( if _is_decode_value then
-      ( ._symbol as $s
-      | if $s != "" then $s end
-      )
-    end
-  );
-
 def _is_scalar:
   type |. != "array" and . != "object";
 
diff --git a/pkg/interp/interp.go b/pkg/interp/interp.go
index cb1111d1..34dbbedb 100644
--- a/pkg/interp/interp.go
+++ b/pkg/interp/interp.go
@@ -871,31 +871,11 @@ func (i *Interp) variables() map[string]interface{} {
 	return variablesAny
 }
 
-func (i *Interp) OptionsEx(fnOptsV ...interface{}) Options {
-	opts := func() Options {
-		vs, err := i.EvalFuncValues(i.evalContext.ctx, nil, "options", []interface{}{fnOptsV}, DiscardCtxWriter{Ctx: i.evalContext.ctx})
-		if err != nil {
-			return Options{}
-		}
-		if len(vs) < 1 {
-			return Options{}
-		}
-		v := vs[0]
-		if _, ok := v.(error); ok {
-			return Options{}
-		}
-		m, ok := v.(map[string]interface{})
-		if !ok {
-			return Options{}
-		}
-		var opts Options
-		_ = mapstructure.Decode(m, &opts)
-		opts.Depth = num.MaxInt(0, opts.Depth)
-
-		return opts
-	}()
-
+func (i *Interp) Options(v interface{}) Options {
+	var opts Options
+	_ = mapstructure.Decode(v, &opts)
 	opts.ArrayTruncate = num.MaxInt(0, opts.ArrayTruncate)
+	opts.Depth = num.MaxInt(0, opts.Depth)
 	opts.AddrBase = num.ClampInt(2, 36, opts.AddrBase)
 	opts.SizeBase = num.ClampInt(2, 36, opts.SizeBase)
 	opts.LineBytes = num.MaxInt(0, opts.LineBytes)
@@ -906,10 +886,6 @@ func (i *Interp) OptionsEx(fnOptsV ...interface{}) Options {
 	return opts
 }
 
-func (i *Interp) Options() Options {
-	return i.OptionsEx(nil)
-}
-
 func (i *Interp) NewColorJSON(opts Options) (*colorjson.Encoder, error) {
 	indent := 2
 	if opts.Compact {
diff --git a/pkg/interp/options.jq b/pkg/interp/options.jq
index 3ba4ffa5..2717c16a 100644
--- a/pkg/interp/options.jq
+++ b/pkg/interp/options.jq
@@ -135,8 +135,6 @@ def _to_options:
   | with_entries(select(.value != null))
   );
 
-# . will have additional array of options taking priority
-# NOTE: is called from go *interp.Interp Options()
 def options($opts):
-  [_build_default_dynamic_options] + _options_stack + $opts | add;
-def options: options([{}]);
+  [_build_default_dynamic_options] + _options_stack + [$opts] | add;
+def options: options({});
diff --git a/pkg/interp/repl.jq b/pkg/interp/repl.jq
index 36f41294..575bee4c 100644
--- a/pkg/interp/repl.jq
+++ b/pkg/interp/repl.jq
@@ -153,16 +153,29 @@ def _prompt:
   , _values
   ] | join(" ") + "> ";
 
-def _repl_display: _display({depth: 1});
+# _repl_display takes a opts arg to make it possible for repl_eval to
+# just call options/0 once per eval even if it was multiple outputs
+def _repl_display_opts: options({depth: 1});
+def _repl_display($opts): _display($opts);
+def _repl_display: _display(_repl_display_opts);
 def _repl_on_error:
   ( if _eval_is_compile_error then _eval_compile_error_tostring
-    # was interrupte by user, just ignore
+    # was interrupted by user, just ignore
     elif _is_context_canceled_error then empty
     end
   | (_error_str | println)
   );
 def _repl_on_compile_error: _repl_on_error;
-def _repl_eval($expr): _eval($expr; "repl"; _repl_display; _repl_on_error; _repl_on_compile_error);
+def _repl_eval($expr):
+  ( _repl_display_opts as $opts
+  | _eval(
+      $expr;
+      "repl";
+      _repl_display($opts);
+      _repl_on_error;
+      _repl_on_compile_error
+    )
+  );
 
 # run read-eval-print-loop
 def _repl($opts): #:: a|(Opts) => @
diff --git a/pkg/interp/testdata/args.fqtest b/pkg/interp/testdata/args.fqtest
index 6b2a3b33..32f723b0 100644
--- a/pkg/interp/testdata/args.fqtest
+++ b/pkg/interp/testdata/args.fqtest
@@ -118,7 +118,6 @@ vpx_ccr              VPX Codec Configuration Record
 wav                  WAV file
 webp                 WebP image
 xing                 Xing header
-zip                  ZIP archive
 $ fq -X
 exitcode: 2
 stderr: