holochain / holochain-rust

DEPRECATED. The Holochain framework implemented in rust with a redux style internal state-model.
GNU General Public License v3.0
1.12k stars 267 forks source link

hdk rust macro issue #567

Open Connoropolous opened 5 years ago

Connoropolous commented 5 years ago

the following doesn't work:

list_tasks: {
                inputs: ||,
                outputs: |tasks: serde_json::Value|,
                handler: handle_list_tasks
            }

only this does:

list_tasks: {
                inputs: | |,
                outputs: |tasks: serde_json::Value|,
                handler: handle_list_tasks
            }

the difference is the space in between the two pipes | |

maackle commented 5 years ago

Also, is there a reason why inputs and outputs need to have names? It's just the types that matter, no? Or does this affect serialization?

maackle commented 5 years ago

One more thing, it's really hard to debug the validation function, many errors often result in the entire macro invocation failing with an obscure and incorrect error:

the size for values of type `dyn std::ops::FnMut(std::string::String, hdk::ValidationData) -> std::result::Result<(), std::string::String> + std::marker::Sync` cannot be known at compilation time

doesn't have a size known at compile-time

help: the trait `std::marker::Sized` is not implemented for `dyn std::ops::FnMut(std::string::String, hdk::ValidationData) -> std::result::Result<(), std::string::String> + std::marker::Sync`
Connoropolous commented 5 years ago

let's open that second one up as a separate issue @maackle

Connoropolous commented 5 years ago

To the point about named inputs and outputs, I feel similarly... especially about the output... it's odd to give it a name, since its always a struct, with the relevant values contained within

Connoropolous commented 5 years ago

@lucksus have you seen this one? Do you agree it's a bug?

lucksus commented 5 years ago

Ah, I have not. Thanks for the mention. Well, I wouldn't consider it a bug. I knew that this wouldn't work when we wrote the macro. If that seems like a problem we could see how we can tweak the macro. The problem I think is that || is an operator, right @sphinxc0re? Or could we change the regex to also match that operator?

We can look into this but I'm removing the bug flag.