bhauman / devcards

Devcards aims to provide a visual REPL experience for ClojureScript
1.53k stars 116 forks source link

Suggest new-style ns declarations for defcard macro #141

Closed pesterhazy closed 5 years ago

pesterhazy commented 5 years ago

The new ":refer-macros" syntax has been available for a while (it's explained here). It makes the README easier to follow.

bhauman commented 5 years ago

There is a reason for the style I'm using here. I'm trying to prevent devcards.core from being a dependency that gets pulled into the dependency tree and included in simple builds.

It's my belief that :require-macros accomplishes this by only making the macros a Clojure runtime dependency.

Granted I regret all this ghost like behavior as it over complicates the library.

On Fri, Jul 13, 2018 at 4:31 AM Paulus Esterhazy notifications@github.com wrote:

The new ":refer-macros" syntax has been available for a while (it's explained here http://blog.fikesfarm.com/posts/2016-03-01-clojurescript-macro-sugar.html). It makes the README easier to follow.

You can view, comment on, or merge this pull request online at:

https://github.com/bhauman/devcards/pull/141 Commit Summary

  • Suggest new-style ns declarations for defcard macro

File Changes

Patch Links:

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/bhauman/devcards/pull/141, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAKQDlpOjbEQ7jeOBuxYDrLelHFEva4ks5uGFrggaJpZM4VOgQD .

pesterhazy commented 5 years ago

Oh, I didn't know there was a deeper reason behind this.

When working on a project, I noticed that it wasn't sufficient to just require-macros the defcard macro; I also had to require the devcard.core namespace, because the ns with the functions needs to be available.

Isn't the following (from the README)

  (:require
   [devcards.core :as dc]) 
  (:require-macros
   [devcards.core :refer [defcard]])

equivalent to my suggestion,

(:require [devcards.core :as dc :refer-macros [defcard]])

though? I mean equivalent in the sense that the latter desugars into the former.

bhauman commented 5 years ago

Yes it's confusing, because you will get warnings if you don't do it the way you mention ... but it will still work as long as the devcards.core namespace is loaded before the namespaces that use it.

Again this is not the best, but it you want cards to disappear when you are not using them this is the only way to do it.

On Fri, Jul 13, 2018 at 9:09 AM Paulus Esterhazy notifications@github.com wrote:

Oh, I didn't know there was a deeper reason behind this.

When working on a project, I noticed that it wasn't sufficient to just require-macros the defcard macro; I also had to require the devcard.core namespace, because the ns with the functions needs to be available.

Isn't the following (from the README)

(:require [devcards.core :as dc]) (:require-macros [devcards.core :refer [defcard]])

equivalent to my suggestion,

(:require [devcards.core :as dc :refer-macros [defcard]])

though? I mean equivalent in the sense that the latter desugars into the former.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/bhauman/devcards/pull/141#issuecomment-404828756, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAKQJTS2LN3zxfrEX2_u4Rl8H4aVmqeks5uGJwbgaJpZM4VOgQD .

bhauman commented 5 years ago

This of course could be made clearer in the documentation. A devcards refactor is slated in the next few months so we can get effortless devcards in figwheel.main. My vision is to just have a "/devcards" endpoint that automagically picks up all the cards.

On Fri, Jul 13, 2018 at 9:23 AM Bruce Hauman bhauman@gmail.com wrote:

Yes it's confusing, because you will get warnings if you don't do it the way you mention ... but it will still work as long as the devcards.core namespace is loaded before the namespaces that use it.

Again this is not the best, but it you want cards to disappear when you are not using them this is the only way to do it.

On Fri, Jul 13, 2018 at 9:09 AM Paulus Esterhazy notifications@github.com wrote:

Oh, I didn't know there was a deeper reason behind this.

When working on a project, I noticed that it wasn't sufficient to just require-macros the defcard macro; I also had to require the devcard.core namespace, because the ns with the functions needs to be available.

Isn't the following (from the README)

(:require [devcards.core :as dc]) (:require-macros [devcards.core :refer [defcard]])

equivalent to my suggestion,

(:require [devcards.core :as dc :refer-macros [defcard]])

though? I mean equivalent in the sense that the latter desugars into the former.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/bhauman/devcards/pull/141#issuecomment-404828756, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAKQJTS2LN3zxfrEX2_u4Rl8H4aVmqeks5uGJwbgaJpZM4VOgQD .

pesterhazy commented 5 years ago

Thanks for the explanation