search

weavejester / hashp

A better "prn" for debugging
MIT License
439 stars 23 forks source link

Support cljs #7

Closed lucywang000 closed 4 years ago

lucywang000 commented 4 years ago

Some notable changes:

  1. Since puget doesn't not support cljs, zprint is used to colorize the forms. Not sure if it's better to remove puget and use zprint in both cljs and clj.
  2. While it is possible to use a single cljc file to serve both clj/cljs, that would make the code too complicated a mental burden, because at reader expansion time the cljc file would be treated as a clj file, while at runtime it's treated as a cljs ns, it's just too confusing. Besides it requires lots of reader conditionals. So imo it's better to keep it as a clj file, and keep the cljs runtime variables like print-opts in a cljs file.
  3. The case macro of net.cgrand/macrovich is used in the p* function to make it generate clj/cljs code properly. Reader conditionals can't work here - this function is always compiled as clj code when being used as a data reader, so the cljs branch would never be used in this case. The macrovich/case macro could help this because it's a macro so it could generate different code for clj/cljs during macro expansion.
weavejester commented 4 years ago

Thanks for the PR. zprint in both cases might be a good idea, as I've had issues with puget, but that should be a separate PR.

I have some standard contributing guidelines that I forgot to add to this repository. Can you squash your commits down and ensure the commit message is formatted according to the guideline?

lucywang000 commented 4 years ago
  1. all commits squashed
  2. drop puget, use zprint in both clj and cljs
  3. formatted commit msg

@weavejester PTAL.

weavejester commented 4 years ago

drop puget, use zprint in both clj and cljs

That's good work, but as I said in my previous comment, that should be a separate PR. Can you revert that change for this PR, as it's beyond the scope of this feature.

lucywang000 commented 4 years ago

@weavejester Reverted the "drop puget" part as you suggested.

lucywang000 commented 4 years ago

@weavejester could you please take a look again? I have been using this ever since and it works pretty well for me.

weavejester commented 4 years ago

Sorry, I've been a little busy with Ring. If the indentation is fixed it should be good to merge.

weavejester commented 4 years ago

Thanks! Can you change the commit message to Add support for ClojureScript? Then I'll merge it in.