js: remove automatic function -> closure conversion

It's a bad idea for several reasons:

* it's inefficient; must allocate an environment for a closure in Nim,
  even though we already have one in JS
* writing macros for automatically creating functions with variadic
  arguments is suprisingly difficult (see the entire `js/javascript'
  module)
* it never really worked properly, because we never freed the associated
  function pointer.

We hardly used it anyway, so the easiest fix is to get rid of it
completely.

9 files changed, 75 insertions, 107 deletions

diff --git a/src/config/config.nim b/src/config/config.nim
index 287c2e54..4f28f4e5 100644
--- a/src/config/config.nim
+++ b/src/config/config.nim
@@ -12,6 +12,7 @@ import config/toml
 import js/error
 import js/fromjs
 import js/javascript
+import js/jstypes
 import js/propertyenumlist
 import js/regex
 import js/tojs
@@ -40,7 +41,7 @@ type
   SiteConfig* = object
     url*: Option[Regex]
     host*: Option[Regex]
-    rewrite_url*: (proc(s: URL): JSResult[URL])
+    rewrite_url*: Option[JSValueFunction]
     cookie*: Option[bool]
     third_party_cookie*: seq[Regex]
     share_cookie_jar*: Option[string]
@@ -54,7 +55,7 @@ type
 
   OmniRule* = object
     match*: Regex
-    substitute_url*: (proc(s: string): JSResult[string])
+    substitute_url*: Option[JSValueFunction]
 
   StartConfig = object
     visual_home* {.jsgetset.}: string
@@ -330,7 +331,7 @@ proc parseConfigValue(ctx: var ConfigParser; x: var Regex; v: TomlValue;
   k: string)
 proc parseConfigValue(ctx: var ConfigParser; x: var URL; v: TomlValue;
   k: string)
-proc parseConfigValue[T](ctx: var ConfigParser; x: var proc(x: T): JSResult[T];
+proc parseConfigValue(ctx: var ConfigParser; x: var JSValueFunction;
   v: TomlValue; k: string)
 proc parseConfigValue(ctx: var ConfigParser; x: var ChaPathResolved;
   v: TomlValue; k: string)
@@ -581,11 +582,16 @@ proc parseConfigValue(ctx: var ConfigParser; x: var URL; v: TomlValue;
     raise newException(ValueError, "invalid URL " & k)
   x = y.get
 
-proc parseConfigValue[T](ctx: var ConfigParser; x: var proc(x: T): JSResult[T];
+proc parseConfigValue(ctx: var ConfigParser; x: var JSValueFunction;
     v: TomlValue; k: string) =
   typeCheck(v, tvtString, k)
   let fun = ctx.config.jsctx.eval(v.s, "<config>", JS_EVAL_TYPE_GLOBAL)
-  x = getJSFunction[T, T](ctx.config.jsctx, fun)
+  if JS_IsException(fun):
+    raise newException(ValueError, "exception in " & k & ": " &
+      ctx.config.jsctx.getExceptionMsg())
+  if not JS_IsFunction(ctx.config.jsctx, fun):
+    raise newException(ValueError, k & " is not a function")
+  x = JSValueFunction(fun: fun)
 
 proc parseConfigValue(ctx: var ConfigParser; x: var ChaPathResolved;
     v: TomlValue; k: string) =
@@ -761,7 +767,7 @@ proc initCommands*(config: Config): Err[string] =
   let obj = JS_NewObject(ctx)
   defer: JS_FreeValue(ctx, obj)
   if JS_IsException(obj):
-    return err(ctx.getExceptionStr())
+    return err(ctx.getExceptionMsg())
   for i in countdown(config.cmd.init.high, 0):
     let (k, cmd) = config.cmd.init[i]
     if k in config.cmd.map:
@@ -776,14 +782,14 @@ proc initCommands*(config: Config): Err[string] =
           prop = JS_NewObject(ctx)
           ctx.definePropertyE(objIt, ss, prop)
         if JS_IsException(prop):
-          return err(ctx.getExceptionStr())
+          return err(ctx.getExceptionMsg())
         objIt = prop
     if cmd == "":
       config.cmd.map[k] = JS_UNDEFINED
       continue
     let fun = ctx.eval(cmd, "<" & k & ">", JS_EVAL_TYPE_GLOBAL)
     if JS_IsException(fun):
-      return err(ctx.getExceptionStr())
+      return err(ctx.getExceptionMsg())
     if not JS_IsFunction(ctx, fun):
       return err(k & " is not a function")
     ctx.definePropertyE(objIt, name, JS_DupValue(ctx, fun))
diff --git a/src/html/dom.nim b/src/html/dom.nim
index 28e3f558..e3d62b60 100644
--- a/src/html/dom.nim
+++ b/src/html/dom.nim
@@ -2846,9 +2846,8 @@ proc reflectEvent(element: Element; target: EventTarget; name: StaticAtom;
   let fun = ctx.newFunction(["event"], value)
   assert ctx != nil
   if JS_IsException(fun):
-    let s = ctx.getExceptionStr()
     document.window.console.log("Exception in body content attribute of",
-      urls, s)
+      urls, ctx.getExceptionMsg())
   else:
     let jsTarget = ctx.toJS(target)
     ctx.definePropertyC(jsTarget, $name, fun)
@@ -3628,13 +3627,12 @@ proc execute*(element: HTMLScriptElement) =
       let urls = script.baseURL.serialize(excludepassword = true)
       let ctx = window.jsctx
       if JS_IsException(script.record):
-        let s = ctx.getExceptionStr()
-        window.console.log("Exception in document", urls, s)
+        window.console.log("Exception in document", urls, ctx.getExceptionMsg())
       else:
         let ret = ctx.evalFunction(script.record)
         if JS_IsException(ret):
-          let s = ctx.getExceptionStr()
-          window.console.log("Exception in document", urls, s)
+          window.console.log("Exception in document", urls,
+            ctx.getExceptionMsg())
         JS_FreeValue(ctx, ret)
     document.currentScript = oldCurrentScript
   else: discard #TODO
diff --git a/src/html/env.nim b/src/html/env.nim
index ebd9e83e..14a12edd 100644
--- a/src/html/env.nim
+++ b/src/html/env.nim
@@ -169,7 +169,7 @@ proc addScripting*(window: Window; selector: Selector[int]) =
       let ret = window.jsctx.eval(src, file, JS_EVAL_TYPE_GLOBAL)
       if JS_IsException(ret):
         window.console.log("Exception in document", $window.document.url,
-          window.jsctx.getExceptionStr())
+          window.jsctx.getExceptionMsg())
       else:
         JS_FreeValue(ctx, ret)
     )
diff --git a/src/js/fromjs.nim b/src/js/fromjs.nim
index a176cd33..f93e89fa 100644
--- a/src/js/fromjs.nim
+++ b/src/js/fromjs.nim
@@ -8,7 +8,6 @@ import io/promise
 import js/error
 import js/jstypes
 import js/opaque
-import js/tojs
 import types/opt
 import utils/twtstr
 
@@ -280,61 +279,6 @@ proc fromJSTable[A, B](ctx: JSContext, val: JSValue): JSResult[Table[A, B]] =
     res[kn] = vn
   return ok(res)
 
-#TODO varargs
-proc fromJSFunction1*[T, U](ctx: JSContext, val: JSValue):
-    proc(x: U): JSResult[T] =
-  #TODO this leaks memory!
-  let dupval = JS_DupValue(ctx, JS_DupValue(ctx, val)) # save
-  return proc(x: U): JSResult[T] =
-    var arg1 = toJS(ctx, x)
-    #TODO exceptions?
-    let ret = JS_Call(ctx, dupval, JS_UNDEFINED, 1, addr arg1)
-    result = fromJS[T](ctx, ret)
-    JS_FreeValue(ctx, ret)
-
-proc isErrType(rt: NimNode): bool =
-  let rtType = rt[0]
-  let errType = getTypeInst(Err)
-  return errType.sameType(rtType) and rtType.sameType(errType)
-
-# unpack brackets
-proc getRealTypeFun(x: NimNode): NimNode =
-  var x = x.getTypeImpl()
-  while true:
-    if x.kind == nnkBracketExpr and x.len == 2:
-      x = x[1].getTypeImpl()
-      continue
-    break
-  return x
-
-macro unpackReturnType(f: typed) =
-  var x = f.getRealTypeFun()
-  let params = x.findChild(it.kind == nnkFormalParams)
-  let rv = params[0]
-  if rv.isErrType():
-    return quote do: void
-  let rvv = rv[1]
-  return quote do: `rvv`
-
-macro unpackArg0(f: typed) =
-  var x = f.getRealTypeFun()
-  let params = x.findChild(it.kind == nnkFormalParams)
-  let rv = params[1]
-  doAssert rv.kind == nnkIdentDefs
-  let rvv = rv[1]
-  return quote do: `rvv`
-
-proc fromJSFunction[T](ctx: JSContext, val: JSValue):
-    JSResult[T] =
-  #TODO all args...
-  if not JS_IsFunction(ctx, val):
-    return err(newTypeError("function expected"))
-  return ok(
-    fromJSFunction1[
-      typeof(unpackReturnType(T)),
-      typeof(unpackArg0(T))
-    ](ctx, val))
-
 template optionType[T](o: type Option[T]): auto =
   T
 
@@ -462,8 +406,6 @@ macro fromJS2(ctx: JSContext; val: JSValue; x: static string): untyped =
 proc fromJS*[T](ctx: JSContext, val: JSValue): JSResult[T] =
   when T is string:
     return fromJSString(ctx, val)
-  elif T is (proc):
-    return fromJSFunction[T](ctx, val)
   elif T is Option:
     return fromJSOption[optionType(T)](ctx, val)
   elif T is seq:
diff --git a/src/js/javascript.nim b/src/js/javascript.nim
index f2cd8a10..ef848038 100644
--- a/src/js/javascript.nim
+++ b/src/js/javascript.nim
@@ -185,7 +185,7 @@ proc free*(rt: var JSRuntime) =
   runtimes.del(runtimes.find(rt))
   rt = nil
 
-proc setGlobal*[T](ctx: JSContext, global: JSValue, obj: T) =
+proc setGlobal*[T](ctx: JSContext; global: JSValue; obj: T) =
   # Add JSValue reference.
   let p = JS_VALUE_GET_PTR(global)
   let header = cast[ptr JSRefCountHeader](p)
@@ -193,10 +193,11 @@ proc setGlobal*[T](ctx: JSContext, global: JSValue, obj: T) =
   ctx.setOpaque(global, cast[pointer](obj))
   GC_ref(obj)
 
-proc setInterruptHandler*(rt: JSRuntime, cb: JSInterruptHandler, opaque: pointer = nil) =
+proc setInterruptHandler*(rt: JSRuntime; cb: JSInterruptHandler;
+    opaque: pointer = nil) =
   JS_SetInterruptHandler(rt, cb, opaque)
 
-proc getExceptionStr*(ctx: JSContext): string =
+proc getExceptionMsg*(ctx: JSContext): string =
   result = ""
   let ex = JS_GetException(ctx)
   let str = fromJS[string](ctx, ex)
@@ -208,11 +209,20 @@ proc getExceptionStr*(ctx: JSContext): string =
   JS_FreeValue(ctx, stack)
   JS_FreeValue(ctx, ex)
 
-proc writeException*(ctx: JSContext, s: DynStream) =
-  s.write(ctx.getExceptionStr())
+proc getExceptionMsg*(ctx: JSContext; err: JSError): string =
+  if err != nil:
+    JS_FreeValue(ctx, ctx.toJS(err)) # note: this implicitly throws
+  return ctx.getExceptionMsg()
+
+proc writeException*(ctx: JSContext; s: DynStream) =
+  s.write(ctx.getExceptionMsg())
+  s.sflush()
+
+proc writeException*(ctx: JSContext; s: DynStream; err: JSError) =
+  s.write(ctx.getExceptionMsg(err))
   s.sflush()
 
-proc runJSJobs*(rt: JSRuntime, err: DynStream) =
+proc runJSJobs*(rt: JSRuntime; err: DynStream) =
   while JS_IsJobPending(rt):
     var ctx: JSContext
     let r = JS_ExecutePendingJob(rt, addr ctx)
@@ -322,10 +332,6 @@ func getMinArgs(params: seq[FuncParam]): int =
 func fromJSP[T: string|uint32](ctx: JSContext, atom: JSAtom): Opt[T] =
   return fromJS[T](ctx, atom)
 
-proc getJSFunction*[T, U](ctx: JSContext, val: JSValue):
-    (proc(x: T): JSResult[U]) =
-  return fromJSFunction1[T, U](ctx, val)
-
 proc defineConsts*[T](ctx: JSContext, classid: JSClassID,
     consts: static openArray[(string, T)]) =
   let proto = ctx.getOpaque().ctors[classid]
diff --git a/src/js/jstypes.nim b/src/js/jstypes.nim
index 5336f067..9e1d72ea 100644
--- a/src/js/jstypes.nim
+++ b/src/js/jstypes.nim
@@ -33,3 +33,11 @@ type
 
 func high*(abuf: JSArrayBuffer): int =
   return int(abuf.len) - 1
+
+# A specialization of JSValue to make writing generic code for functions
+# easier.
+type JSValueFunction* = ref object
+  fun*: JSValue
+
+converter toJSValue*(f: JSValueFunction): JSValue =
+  f.fun
diff --git a/src/local/pager.nim b/src/local/pager.nim
index 4d29f4f0..624a4867 100644
--- a/src/local/pager.nim
+++ b/src/local/pager.nim
@@ -3,14 +3,12 @@ import std/net
 import std/options
 import std/os
 import std/osproc
+import std/posix
 import std/selectors
 import std/streams
 import std/tables
 import std/unicode
 
-when defined(posix):
-  import std/posix
-
 import bindings/libregexp
 import config/config
 import config/mailcap
@@ -23,6 +21,7 @@ import io/stdio
 import io/tempfile
 import io/urlfilter
 import js/error
+import js/fromjs
 import js/javascript
 import js/jstypes
 import js/regex
@@ -115,6 +114,7 @@ type
     inputBuffer*: string # currently uninterpreted characters
     iregex: Result[Regex, string]
     isearchpromise: EmptyPromise
+    jsctx: JSContext
     lineData: LineData
     lineedit*: Option[LineEdit]
     linehist: array[LineMode, LineHistory]
@@ -282,7 +282,8 @@ proc newPager*(config: Config; forkserver: ForkServer; ctx: JSContext;
     config: config,
     forkserver: forkserver,
     term: newTerminal(stdout, config),
-    alerts: alerts
+    alerts: alerts,
+    jsctx: ctx
   )
 
 proc genClientKey(pager: Pager): ClientKey =
@@ -865,15 +866,25 @@ proc applySiteconf(pager: Pager; url: var URL; charsetOverride: Charset;
   var charsets = pager.config.encoding.document_charset
   var userstyle = pager.config.css.stylesheet
   var proxy = pager.config.network.proxy
+  let ctx = pager.jsctx
   for sc in pager.config.siteconf:
     if sc.url.isSome and not sc.url.get.match($url):
       continue
     elif sc.host.isSome and not sc.host.get.match(host):
       continue
-    if sc.rewrite_url != nil:
-      let s = sc.rewrite_url(url)
-      if s.isSome and s.get != nil:
-        url = s.get
+    if sc.rewrite_url.isSome:
+      let fun = sc.rewrite_url.get
+      var arg1 = ctx.toJS(url)
+      let ret = JS_Call(ctx, fun, JS_UNDEFINED, 1, addr arg1)
+      let nu = fromJS[URL](ctx, ret)
+      if nu.isOk:
+        if nu.get != nil:
+          url = nu.get
+      elif JS_IsException(ret):
+        #TODO should writeException the message to console
+        pager.alert("Error rewriting URL: " & ctx.getExceptionMsg(nu.error))
+      JS_FreeValue(ctx, arg1)
+      JS_FreeValue(ctx, ret)
     if sc.cookie.isSome:
       if sc.cookie.get:
         # host/url might have changed by now
@@ -964,12 +975,15 @@ proc gotoURL(pager: Pager; request: Request; prevurl = none(URL);
 proc omniRewrite(pager: Pager, s: string): string =
   for rule in pager.config.omnirule:
     if rule.match.match(s):
-      let sub = rule.substitute_url(s)
-      if sub.isSome:
-        return sub.get
-      else:
-        let buf = $rule.match
-        pager.alert("Error in substitution of rule " & buf & " for " & s)
+      let fun = rule.substitute_url.get
+      let ctx = pager.jsctx
+      var arg1 = ctx.toJS(s)
+      let jsRet = JS_Call(ctx, fun, JS_UNDEFINED, 1, addr arg1)
+      let ret = fromJS[string](ctx, jsRet)
+      if ret.isOk:
+        return ret.get
+      pager.alert("Error in substitution of " & $rule.match & " for " & s &
+        ": " & ctx.getExceptionMsg(ret.error))
   return s
 
 # When the user has passed a partial URL as an argument, they might've meant
diff --git a/src/server/buffer.nim b/src/server/buffer.nim
index 363f431c..a3e4afe2 100644
--- a/src/server/buffer.nim
+++ b/src/server/buffer.nim
@@ -1016,8 +1016,7 @@ proc dispatchDOMContentLoadedEvent(buffer: Buffer) =
     if el.ctype == "DOMContentLoaded":
       let e = ctx.invoke(el, event)
       if JS_IsException(e):
-        buffer.estream.write(ctx.getExceptionStr())
-        buffer.estream.sflush()
+        ctx.writeException(buffer.estream)
       JS_FreeValue(ctx, e)
       called = true
       if FLAG_STOP_IMMEDIATE_PROPAGATION in event.flags:
@@ -1039,8 +1038,7 @@ proc dispatchLoadEvent(buffer: Buffer) =
     if el.ctype == "load":
       let e = ctx.invoke(el, event)
       if JS_IsException(e):
-        buffer.estream.write(ctx.getExceptionStr())
-        buffer.estream.sflush()
+        ctx.writeException(buffer.estream)
       JS_FreeValue(ctx, e)
       called = true
       if FLAG_STOP_IMMEDIATE_PROPAGATION in event.flags:
@@ -1069,8 +1067,7 @@ proc dispatchEvent(buffer: Buffer; ctype, jsName: string; elem: Element):
         let e = ctx.invoke(el, event)
         called = true
         if JS_IsException(e):
-          buffer.estream.write(ctx.getExceptionStr())
-          buffer.estream.sflush()
+          ctx.writeException(buffer.estream)
         JS_FreeValue(ctx, e)
         if FLAG_STOP_IMMEDIATE_PROPAGATION in event.flags:
           stop = true
@@ -1494,8 +1491,7 @@ proc evalJSURL(buffer: Buffer, url: URL): Opt[string] =
   let ctx = buffer.window.jsctx
   let ret = ctx.eval(scriptSource, $buffer.baseURL, JS_EVAL_TYPE_GLOBAL)
   if JS_IsException(ret):
-    buffer.estream.write(ctx.getExceptionStr())
-    buffer.estream.sflush()
+    ctx.writeException(buffer.estream)
     return err() # error
   if JS_IsUndefined(ret):
     return err() # no need to navigate
diff --git a/todo b/todo
index 9c814182..8b99ef9f 100644
--- a/todo
+++ b/todo
@@ -50,8 +50,6 @@ network:
 - uBO integration? (or at least implement filter lists)
 - websockets (curl supports ws)
 javascript:
-- important: callbacks should not leak memory
-	* we should probably just remove automatic function conversion
 - add support for JS mixins
 - distinguish double from unrestricted double
 - better dom support: more events, CSSOM, ...