Skip to content
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

Fix broken instrumentation of forms with #? conditionals #435

Merged
merged 2 commits into from
Sep 13, 2017

Conversation

vspinu
Copy link
Contributor

@vspinu vspinu commented Aug 19, 2017

Any forms that contain reader conditionals cannot be instrumented:

(defn tt [a]
  (let [b (+ a 1)]
    #?(:clj (println "clj")
       :cljs (println "cljs"))))

The reason is that conditionals are disabled by default in clojure's readers.

@bbatsov
Copy link
Member

bbatsov commented Aug 19, 2017

I guess this should also have some test and a changelog entry on CIDER's side ideally.

@Malabarba
Copy link
Member

Stepping through these forms probably won't work when they're slightly more complicated, but it's better than the current situation of not reading them at all.

  New lines in REPL throw "end of input" which makes it harder to debug that
  function.
@vspinu
Copy link
Contributor Author

vspinu commented Aug 20, 2017

Indeed, debugger breaks on more complex inputs. Opening a separate #437 for that.

@bbatsov there is little point of testing whether debugger enters the form or not. More specific tests of #437 will do that automatically. I will add the log entry on the cider side for multiple fixes at once.

@@ -571,7 +571,9 @@ this map (identified by a key), and will `dissoc` it afterwards."}
(interleave '[dbg break light])
(apply assoc *data-readers*))]
(try
(read-string {:read-cond :allow} code)
;; new-line in REPL always throws; skip for debug convenience
(when (> (count code) 3)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we check for them explicitly? Seems a bit arbitrary to check that something is 3+ characters.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not completely sure what you mean but we do check for all of them explicitly with that fake reader above. All of them have >= 4 chars this is why 3 characters threshold. It's indeed arbitrary but avoids entering that code in simple cases when there is obviously no reader there. It could be 6 there just as fine :).

As mentioned in the comment, new line in REPL will throw and trigger that catch over there. When debugging I am placing a print there to see exactly what is going on, and those useless throws interfere with the experience. Does this clarify your question?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, I guess you can just write more explanations here and that would be enough. I just assume that if some code is confusing to me it will probably be confusing to someone else as well.

@bbatsov bbatsov merged commit 1e1cee1 into clojure-emacs:master Sep 13, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants