mfikes / ambly

ClojureScript REPL into embedded JavaScriptCore
http://ambly.fikesfarm.com
Eclipse Public License 1.0
540 stars 21 forks source link

Stackframes for REPL-defined functions #19

Closed mfikes closed 9 years ago

mfikes commented 9 years ago

@swannodette Considering a potential enhancement. Could use feedback. Low priority IMHO.

A user could define functions in the REPL that call into functions that are defined in source and then call those REPL-defined functions. If an exception occurs, JSC will emit only the function names in frames for REPL-defined functions.

The Clojure REPL supports this, using NO_SOURCE_FILE and line 1.

To support this would require minor changes to Ambly, and some changes to the underlying ClojureScript REPL as well.

Here is a concrete example showing that it almost works:

In actual source, I have:

(ns hello-ambly.core)

;; some lines have been deleted from here for brevity, 
;; so stack line numbers below are off

(defn test-exception-two [] 
  (ffirst 1))

(defn test-exception-one []
  (test-exception-two))

And in the Ambly REPL:

(require 'hello-ambly.core)
(defn wrap-two [] (hello-ambly.core/test-exception-one))
(defn wrap-one [] (wrap-two))

Here is it nearly working:

ClojureScript:cljs.user> (wrap-one)
Error: 1 is not ISeqable
     cljs.core/seq (out/cljs/core.cljs:727:13)
     cljs.core/first (out/cljs/core.cljs:736:7)
     cljs.core/ffirst (out/cljs/core.cljs:1155:3)
     hello-ambly.core/test-exception-two (out/hello_ambly/core.cljs:16:3)
     hello-ambly.core/test-exception-one (out/hello_ambly/core.cljs:19:3)
     wrap_two (out/NO_SOURCE_FILE:1:1)
     wrap_one (out/NO_SOURCE_FILE:1:1)
nil

The two things wrong:

  1. wrap_one should be resolved to cljs.user/wrap-one 2, NO_SOURCE_FILE needs a little special handling to prevent code assuming it is a file in the :output-dir: out/NO_SOURCE_FILE.

I did the above by changing Ambly's parser a little to emit:

[{:file "cljs/core.js", :function "seq", :line 4212, :column 17}
 {:file "cljs/core.js", :function "first", :line 4242, :column 22}
 {:file "cljs/core.js", :function "ffirst", :line 5292, :column 39} 
{:file "hello_ambly/core.js", :function "test_exception_two", :line 19, :column 29}
 {:file "hello_ambly/core.js", :function "test_exception_one", :line 22, :column 48} 
{:file "NO_SOURCE_FILE", :function "wrap_two", :line 1, :column 1} 
{:file "NO_SOURCE_FILE", :function "wrap_one", :line 1, :column 1}]
(defn stack-line->canonical-frame
  "Parses a stack line into a frame representation, returning nil
  if parse failed."
  [stack-line opts]
  (let [[function file line column]
        (rest (re-matches #"(.*)@file://(.*):([0-9]+):([0-9]+)"
                stack-line))]
    (if-not (#{"" "global code"} stack-line)
      {:file     (if file
                   (string/replace
                     (.getCanonicalFile (io/file file))
                     (str (System/getProperty "user.dir") File/separator
                       (util/output-directory opts) File/separator)
                     "")
                   "NO_SOURCE_FILE")
       :function (or function (string/trim stack-line))
       :line     (if line (Long/parseLong line) 1)
       :column   (if column (Long/parseLong column) 1)})))

This is a fairly lengthy description and probably needs some hammock time on whether to even pursue it, so I figured I'd drop it in an enhancement ticket here.

swannodette commented 9 years ago

@mfikes ClojureScript enhancement ticket welcome

mfikes commented 9 years ago

CLJS-1034

swannodette commented 9 years ago

Fixed https://github.com/omcljs/ambly/commit/0a13bbe8766474b164b7af763d467505d4434ebd