Remove diagnostics from default features #379
Conversation
hydroflow/Cargo.toml
Outdated
| # Diagnostics feature does not work on wasm32. | ||
| [target.'cfg(not(target_arch = "wasm32"))'.dependencies] | ||
| hydroflow_lang = { path = "../hydroflow_lang", features = ["diagnostics"] } |
There was a problem hiding this comment.
Shouldn't this be passed thru hydroflow_macro ?
There was a problem hiding this comment.
Arbitrarily chose hydroflow_lang since it was the only one not marked as optional. Can change to hydroflow_macro if you prefer that, I have no opinion.
There was a problem hiding this comment.
Hmm yeah, I think we should have the feature in hydroflow_macro as well and go thru that, only have diagnostics enabled if the macros feature is enabled
There was a problem hiding this comment.
Hmm even when targeting wasm32 in hydroflow we should have diagnostics enabled, right? Because those diagnostics are for the macro which isn't being compiled to wasm32 regardless...
There was a problem hiding this comment.
That's what I was thinking, if wasm is only the target we should still be able to have diagnostics
There was a problem hiding this comment.
Although, we don't even need these lines because when we build hydroflow_datalog, we will enable the diagnostic features anyway. So I think I can remove these lines from hydroflow/Cargo.toml entirely.
There was a problem hiding this comment.
When we do [the macro parsing] compilation [step] in browser e.g. for the demo, that is when diagnostics dont work
|
This is just for compiling on your computer with wasm as a target, right? Why don't diagnostics work again? |
|
Yeah I am slightly confused. I get disabling the default feature so that the core crates compile, but we should leave the feature on when we are depending on them from a proc macro crate because we aren't actually compiling to |
Diagnostics is still enabled for hydroflow and hydroflow_data when building non-wasm32 targets.
| @@ -3,6 +3,10 @@ name = "hydroflow_macro" | |||
| version = "0.1.0" | |||
| edition = "2021" | |||
|
|
|||
| [features] | |||
| default = [] | |||
| diagnostics = [ "hydroflow_lang/diagnostics" ] | |||
There was a problem hiding this comment.
So because this is a proc macro, diagnostics should just always be on rather than behind a propagated feature.
|
Closing in favor of #414 |
Diagnostics is still enabled for hydroflow and hydroflow_data when building non-wasm32 targets.
PR for #329.