From: jaawerth <jaawerth@gmail.com>

Also updates fennel.repl to be a callable table of overrdies for the
default options.

Originally, `assert-repl` was intended as a drop-in replacement for
`assert`, but the current implementation treats the third argument as
opts for `fennel.repl`, and discards any additional arguments, whereas
assert passes everything through upon success.

This could lead to unexpected bugs when using `--assert-as-repl`: a
common Lua pattern is to either `return v, ...` or `return nil, err,
...` depending on error state, which assert will naturally convert back
into a normal error pattern; similarly, assert and pcall a
complementary.

This patch throws away the `opts` argument, making `assert-repl` a true
drop-in replacement by ensuring all arguments to assert are returned
unless the assertion ails.

** Overriding REPL callbacks in custom environments **

Since we no longer accept an opts table, we need a way to override the
REPL's defaults for assert-repl: instead of relying on a magic global
like _G.__repl__, default opts for fennel.repl can now be overridden by
setting the fields on fennel.repl (now a callable table) itself:

(set fennel.repl.readChunk custom-input-fn)

Now, `(assert-repl (foo))`, `(fennel.repl)`, `(fennel.repl {})`, etc
will all use `custom-input-fn` as the new default readChunk callback.

To invoke the REPL without invoking overrides, one can call the inner
repl function as (fennel.repl.repl).
---
 api.md                | 16 ++++++++++++++++
 changelog.md          |  7 ++++++-
 reference.md          | 32 ++++++++++++++++++++++----------
 src/fennel/macros.fnl | 28 +++++++++++++++-------------
 src/fennel/repl.fnl   |  4 ++++
 test/api.fnl          | 16 ++++++++++++----
 test/macro.fnl        | 43 +++++++++++++++++++++----------------------
 7 files changed, 96 insertions(+), 50 deletions(-)

diff --git a/api.md b/api.md
index db567fd..53a20c0 100644
--- a/api.md
+++ b/api.md
@@ -74,6 +74,22 @@ Takes these additional options:
 By default, metadata will be enabled and you can view function signatures and
 docstrings with the `,doc` command in the REPL.
 
+### Customize REPL default options
+
+Any fields set on `fennel.repl`, which is actually a table with a `__call`
+metamethod rather than a function, will used as a fallback for any options
+passed to `(fennel.repl)` before defaults are applied, allowing one to
+customize the default behavior of `(fennel.repl)`:
+
+```lua
+fennel.repl.onError = custom_error_handler
+-- In rare cases this needs to be temporary, overrides
+-- can be cleared by simply clearing the entire table
+for k in pairs(fennel.repl) do
+  fennel.repl[k] = nil
+end
+```
+
 ## Evaluate a string of Fennel
 
 ```lua
diff --git a/changelog.md b/changelog.md
index aaec45d..4aef0f6 100644
--- a/changelog.md
+++ b/changelog.md
@@ -10,7 +10,12 @@ deprecated forms.
 
 ### New Features
 
-* ???
+* `fennel.repl` is now a callable table, allowing the default `(fennel.repl)`
+  options to be customized by setting option fields on the table itself.
+
+### Bug Fixes
+
+* `assert-repl`, as a drop-in replacement for `assert`, no longer takes an `opts` param
 
 ## 1.4.0 / 2023-12-01
 
diff --git a/reference.md b/reference.md
index 38ea3f8..543abc8 100644
--- a/reference.md
+++ b/reference.md
@@ -1506,16 +1506,10 @@ use the `assert-repl` macro to do this:
   (assert-repl (transform helper value) "could not transform"))
 ```
 
-This works like the built-in `assert` function, but when the condition
-is false or nil, instead of an error, it drops into a repl which
-has access to all the locals that are in scope. (This would be
-`input`, `value`, and `helper` in the example above.) It takes an
-optional options table as its third argument which accepts all the same
-values as the `fennel.repl` function in the API.
-
-You can `,return EXPRESSION` from the repl to replace the original
-failing condition with a different arbitrary value. Returning false or
-nil will trigger a regular `assert` failure.
+This works as a drop-in replacement for the built-in `assert` function, but
+when the condition is false or nil, instead of an error, it drops into a repl
+which has access to all the locals that are in scope (`input`, `value`, and
+`helper` in the example above).
 
 Note that this is meant for use in development and will not work with
 ahead-of-time compilation unless your build also includes Fennel as a
@@ -1524,6 +1518,24 @@ library.
 If you use the `--assert-as-repl` flag when running Fennel, calls to
 `assert` will be replaced with `assert-repl` automatically.
 
+**Note:** In Fennel 1.4.0, `assert-repl` accepted an options table for
+`fennel.repl` as an optional third argument. This was removed as a bug in
+1.4.1, as it broke compatibility with `assert`.
+
+The REPL spawned by `assert-repl` applies the same default options as
+`fennel.repl`, which as of Fennel 1.4.1 can be configured from the API. See the
+[Fennel API reference](api.md#customize-repl-default-options) for details.
+
+#### Recovering from failed assertions
+
+You can `,return EXPRESSION` from the repl to replace the original
+failing condition with a different arbitrary value. Returning false or
+nil will trigger a regular `assert` failure.
+
+**Note:** Currently, only a single value can be returned from the REPL this
+way. While `,return` can be used to make a failed assertion recover, if the
+calling code expects multiple return values, it may cause unspecified
+behavior.
 
 ## Macros
 
diff --git a/src/fennel/macros.fnl b/src/fennel/macros.fnl
index 6c81710..7ad7cb3 100644
--- a/src/fennel/macros.fnl
+++ b/src/fennel/macros.fnl
@@ -395,29 +395,31 @@ Example:
             (tset scope.macros import-key (. macros* macro-name))))))
   nil)
 
-(fn assert-repl* [condition message ?opts]
-  "Drop into a debug repl and print the message when condition is false/nil.
-Takes an optional table of arguments which will be passed to fennel.repl."
+(fn assert-repl* [condition ...]
+  "Enter into a debug REPL  and print the message when condition is false/nil.
+Works as a drop-in replacement for Lua's `assert`.
+REPL `,return` command returns values to assert in place to continue execution."
+  {:fnl/arglist [condition ?message ...]}
   (fn add-locals [{: symmeta : parent} locals]
     (each [name (pairs symmeta)]
       (tset locals name (sym name)))
     (if parent (add-locals parent locals) locals))
-  `(let [condition# ,condition
-         message# (or ,message "assertion failed, entering repl.")]
+  `(let [unpack# (or table.unpack _G.unpack)
+         pack# (or table.pack #(doto [$...] (tset :n (select :# $...))))
+         ;; need to pack/unpack input args to account for (assert (foo)),
+         ;; because assert returns *all* arguments upon success
+         vals# (pack# ,condition ,...)
+         condition# (. vals# 1)
+         message# (or (. vals# 2) "assertion failed, entering repl.")]
      (if (not condition#)
-         (let [opts# (or ,?opts {:assert-repl? true
-                                 :readChunk (?. _G :___repl___ :readChunk)
-                                 :onError (?. _G :___repl___ :onError)
-                                 :onValued (?. _G :___repl___ :onValued)})
+         (let [opts# {:assert-repl? true}
                fennel# (require (or opts#.moduleName :fennel))
                locals# ,(add-locals (get-scope) [])]
            (set opts#.message (fennel#.traceback message#))
            (set opts#.env (collect [k# v# (pairs _G) &into locals#]
                             (if (= nil (. locals# k#)) (values k# v#))))
-           (_G.assert (fennel#.repl opts#) message#))
-         ;; `assert` returns *all* params on success, but omitting opts# to
-         ;; defensively prevent accidental leakage of REPL opts into code
-         (values condition# message#))))
+           (_G.assert (fennel#.repl opts#)))
+         (values (unpack# vals# 1 vals#.n)))))
 
 {:-> ->*
  :->> ->>*
diff --git a/src/fennel/repl.fnl b/src/fennel/repl.fnl
index bb6078c..895bb2e 100644
--- a/src/fennel/repl.fnl
+++ b/src/fennel/repl.fnl
@@ -454,3 +454,7 @@ For more information about the language, see https://fennel-lang.org/reference")
         (readline.save_history))
       (when opts.exit (opts.exit opts depth))
       value)))
+
+(setmetatable {} {:__call (fn [overrides ?opts]
+                            (repl (utils.copy ?opts (utils.copy overrides))))
+                  :__index {: repl}})
diff --git a/test/api.fnl b/test/api.fnl
index f1fb3b5..c128594 100644
--- a/test/api.fnl
+++ b/test/api.fnl
@@ -21,7 +21,7 @@
   :multi-sym?      "function"
   :parser          "function"
   :path            "string"
-  :repl            "function"
+  :repl            "callable"
   :runtime-version "function"
   :search-module   "function"
   :searcher        "function"
@@ -61,6 +61,14 @@
   :stringStream  "function"
   :unmangle      "function"})
 
+(fn supertype [expect v]
+  (let [vt (type v)]
+    (if (and (= expect :callable)
+             (or (= vt :function) (and (= vt :table)
+                                       (?. (getmetatable v) :__call))))
+      :callable
+      vt)))
+
 (fn test-api-exposure []
   (let [fennel (require :fennel) current {}]
 
@@ -69,17 +77,17 @@
 
     (each [key kind (pairs expected)]
       (t.is (. fennel key) (.. "expect fennel." key " to exists"))
-      (t.= (type (. fennel key)) kind
+      (t.= (supertype kind (. fennel key)) kind
            (.. "expect fennel." key " to be \"" kind "\"")))
 
     (each [key kind (pairs expected-aliases)]
       (t.is (. fennel key) (.. "expect alias fennel." key " to exists"))
-      (t.= (type (. fennel key)) kind
+      (t.= (supertype kind (. fennel key)) kind
            (.. "expect alias fennel." key " to be \"" kind "\"")))
 
     (each [key kind (pairs expected-deprecations)]
       (t.is (. fennel key) (.. "expect deprecated fennel." key " to exists"))
-      (t.= (type (. fennel key)) kind
+      (t.= (supertype kind (. fennel key)) kind
            (.. "expect deprecated fennel." key " to be \"" kind "\"")))
 
     (each [key value (pairs fennel)]
diff --git a/test/macro.fnl b/test/macro.fnl
index 9cfbcda..cb7af60 100644
--- a/test/macro.fnl
+++ b/test/macro.fnl
@@ -739,45 +739,44 @@
   (set _G.x 3)
   (let [inputs ["x\n" "(inc x)\n" "(length hello)\n" ",return 22\n"]
         outputs []
-        env (setmetatable {:print #(table.insert outputs $)
-                           :io {:read #(table.remove inputs 1)
-                                :stderr {:write #(table.insert outputs $2)}}}
-                          {:__index _G})
+        _ (do (set fennel.repl.readChunk #(table.remove inputs 1))
+              (set fennel.repl.onValues (fn [[x]] (table.insert outputs x))))
         form (view (let [hello :world]
                      (fn inc [x] (+ x 1))
-                     (fn onValues [[x]] (print x))
                      (fn g [x]
-                       (assert-repl (< x 2000) "AAAAAH" {:readChunk io.read
-                                                         : onValues}))
+                       (assert-repl (< x 2000) "AAAAAH" "WAHHHH"))
                      (fn f [x] (g (* x 2)))
                      (f 28)
                      (f 1010)))]
-    (t.= [true 22 "AAAAAH"]
-         [(pcall fennel.eval form {: env})])
+    (t.= [true 22]
+         [(pcall fennel.eval form)])
     (t.= [] inputs)
     (t.= ["AAAAAH" "2020" "2021" "5" "22"]
          [(string.gsub (. outputs 1) "%s*stack traceback:.*" "")
           (unpack outputs 2)])
-    (t.= (assert-repl :a-string) :a-string)
-    (let [env (setmetatable {:io {:read #",return nil" :stderr {:write #nil}}}
-                            {:__index _G})
-          form (view (assert-repl false "oh no" {:readChunk io.read
-                                                 :onValues #nil}))
-          (ok? msg) (pcall fennel.eval form {: env})]
+    (t.= [(assert-repl :a-string :b-string :c-string)] [:a-string :b-string :c-string])
+    ;; Set REPL to return immediately for next assertions
+    (set fennel.repl.onError #nil)
+    (set fennel.repl.onValues #",return nil")
+    (let [form (view (assert-repl false "oh no"))
+          multi-args-form (view (assert-repl (select 1 :a :b nil nil :c)))
+          (ok? msg) (pcall fennel.eval form)]
       (t.= false ok? "assertion should fail from repl when returning nil")
+      (t.= [true :a :b nil nil :c] [(pcall fennel.eval multi-args-form)]
+           "assert-repl should pass along all runtime ret vals upon success")
       (t.match "oh no" msg))))
 
 (fn test-assert-as-repl []
-  (let [env (setmetatable {:io {:read #",return :nerevar" :stderr {:write #nil}}}
-                          {:__index _G})
-        ;; TODO: move opts out of the assert call!
-        form (view (assert nil "you nwah" {:readChunk io.read
-                                           :onValues #nil}))
-        (ok? val) (pcall fennel.eval form {: env :assertAsRepl true})]
+  (set fennel.repl.readChunk #",return :nerevar")
+  (set fennel.repl.onValues #nil)
+  (let [form (view (assert nil "you nwah"))
+        (ok? val) (pcall fennel.eval form {:assertAsRepl true})]
     (t.is ok? "should be able to recover from nil assertion.")
     (t.= "nerevar" val)))
 
-{: test-arrows
+{:teardown #(each [k (pairs fennel.repl)]
+              (tset fennel.repl k nil))
+ : test-arrows
  : test-doto
  : test-?.
  : test-import-macros
-- 
2.38.5

From: jaawerth <jaawerth@gmail.com>

There are a few places where we've internally referenced the active
fennel module name for dynamically requiring fennel during runtime,
accounting for a non-standard module name. This adds a
`fennel-module-name` helper to the compiler env to serve this purpose,
allowing it to be accessed from macros.

Also updated `assert-repl` (which prompted this change), function
metadata insertion, and `with-open` to use the helper.

This change theoretically opens the path to userland macros
conditionally accessing the fennel API even when it's been loaded via an
alternate module path, but we'd require further changes to make it
usable, as the fennel module name isn't automatically detected and is
assumed to be `fennel` unless the undocumented `moduleName` option
(which should stay undocumented to avoid confusion with the
also-undocumented 'module-name') is passed.

Holding off on documenting this for now until we can ensure fully
supported usage, with automatic detection of module name when fennel is
required.
---
 src/fennel/macros.fnl   | 5 +++--
 src/fennel/specials.fnl | 6 ++++--
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/src/fennel/macros.fnl b/src/fennel/macros.fnl
index 7ad7cb3..b8917f5 100644
--- a/src/fennel/macros.fnl
+++ b/src/fennel/macros.fnl
@@ -101,7 +101,8 @@ encountering an error before propagating it."
                   ,...)
         closer `(fn close-handlers# [ok# ...]
                   (if ok# ... (error ... 0)))
-        traceback `(. (or package.loaded.fennel debug) :traceback)]
+        traceback `(. (or (. package.loaded ,(fennel-module-name)) debug)
+                      :traceback)]
     (for [i 1 (length closable-bindings) 2]
       (assert (sym? (. closable-bindings i))
               "with-open only allows symbols in bindings")
@@ -413,7 +414,7 @@ REPL `,return` command returns values to assert in place to continue execution."
          message# (or (. vals# 2) "assertion failed, entering repl.")]
      (if (not condition#)
          (let [opts# {:assert-repl? true}
-               fennel# (require (or opts#.moduleName :fennel))
+               fennel# (require (fennel-module-name))
                locals# ,(add-locals (get-scope) [])]
            (set opts#.message (fennel#.traceback message#))
            (set opts#.env (collect [k# v# (pairs _G) &into locals#]
diff --git a/src/fennel/specials.fnl b/src/fennel/specials.fnl
index 1370952..aea0180 100644
--- a/src/fennel/specials.fnl
+++ b/src/fennel/specials.fnl
@@ -35,6 +35,8 @@ will see its values updated as expected, regardless of mangling rules."
 
                             (values next (utils.kvmap env putenv) nil))}))
 
+(fn fennel-module-name [] (or utils.root.options.moduleName :fennel))
+
 (fn current-global-names [?env]
   ;; if there's a metatable on ?env, we need to make sure it's one that has a
   ;; __pairs metamethod, otherwise we give up entirely on globals checking.
@@ -227,8 +229,7 @@ the number of expected arguments."
         (if (= k :fnl/arglist)
             (insert-arglist meta-fields v)
             (insert-meta meta-fields k v)))
-      (let [meta-str (: "require(\"%s\").metadata" :format
-                        (or utils.root.options.moduleName :fennel))]
+      (let [meta-str (: "require(\"%s\").metadata" :format (fennel-module-name))]
         (compiler.emit parent
                        (: "pcall(function() %s:setall(%s, %s) end)" :format
                           meta-str fn-name (table.concat meta-fields ", ")))))))
@@ -1110,6 +1111,7 @@ Only works in Lua 5.3+ or LuaJIT with the --use-bit-lib flag.")
              : unpack
              :assert-compile compiler.assert
              : view
+             : fennel-module-name
              :version utils.version
              ;; AST functions
              :ast-source utils.ast-source
-- 
2.38.5

fennel/patches/.build.yml: FAILED in 12s

[assert-repl: fix as drop-in replacement for assert][0] from [~jaawerth][1]

[0]: https://lists.sr.ht/~technomancy/fennel/patches/48905
[1]: jaawerth@gmail.com

✗ #1138044 FAILED fennel/patches/.build.yml https://builds.sr.ht/~technomancy/job/1138044

~jaawerth <jaawerth@git.sr.ht> writes:

> +  `(let [unpack# (or table.unpack _G.unpack)
> +         pack# (or table.pack #(doto [$...] (tset :n (select :# $...))))
> +         ;; need to pack/unpack input args to account for (assert (foo)),
> +         ;; because assert returns *all* arguments upon success

Overall this patch looks good; thanks for submitting.

However, we went back and forth about whether this pack was necessary,
and I think it is, but if you remove it, the tests still pass.

I'll go ahead and apply the patch as-is, but could you follow up with a
test for that case so we know we're covered? Since it's kind of a subtle
point.

-Phil

Phil Hagelberg <phil@hagelb.org> writes:

> I'll go ahead and apply the patch as-is, but could you follow up with a
> test for that case so we know we're covered? Since it's kind of a subtle
> point.

Oops, spoke to soon. There was a bug in the tests that only showed up
with LuaJIT. Pushed a fix:

https://git.sr.ht/~technomancy/fennel/commit/fcc04a75f8331100d57cb5ee49a531352d00090f

-Phil

Good call - I'll push a follow up commit adding some test cases
asserting against what `pack` is there to handle.

Also, thanks for pushing that fix. Thought I'd caught those, but I guess
not!

-Jesse

On Fri, Feb 2, 2024 at 12:08 PM Phil Hagelberg <phil@hagelb.org> wrote:
>
> ~jaawerth <jaawerth@git.sr.ht> writes:
>
> > +  `(let [unpack# (or table.unpack _G.unpack)
> > +         pack# (or table.pack #(doto [$...] (tset :n (select :# $...))))
> > +         ;; need to pack/unpack input args to account for (assert (foo)),
> > +         ;; because assert returns *all* arguments upon success
>
> Overall this patch looks good; thanks for submitting.
>
> However, we went back and forth about whether this pack was necessary,
> and I think it is, but if you remove it, the tests still pass.
>
> I'll go ahead and apply the patch as-is, but could you follow up with a
> test for that case so we know we're covered? Since it's kind of a subtle
> point.
>
> -Phil