jazzdotdev / jazz

The Scripting Engine that Combines Speed, Safety, and Simplicity
Apache License 2.0
146 stars 11 forks source link

patch-rs binding #160

Closed juchiast closed 5 years ago

juchiast commented 5 years ago

I'm using git dependency for patch-rs, because v0.2 is not published yet.

juchiast commented 5 years ago

@dariusc93

I tried that, but the compiler complains:

error[E0277]: `(dyn std::error::Error + std::marker::Send + 'static)` cannot be shared between threads safely
 --> src/bindings/app/patch.rs:8:38
  |
8 |             this.0.process().map_err(rlua::Error::external)
  |                                      ^^^^^^^^^^^^^^^^^^^^^ `(dyn std::error::Error + std::marker::Send + 'static)` cannot be shared between threads safely
  |
  = help: the trait `std::marker::Sync` is not implemented for `(dyn std::error::Error + std::marker::Send + 'static)`
  = note: required because of the requirements on the impl of `std::marker::Sync` for `std::ptr::Unique<(dyn std::error::Error + std::marker::Send + 'static)>`
  = note: required because it appears within the type `std::boxed::Box<(dyn std::error::Error + std::marker::Send + 'static)>`
  = note: required because it appears within the type `std::option::Option<std::boxed::Box<(dyn std::error::Error + std::marker::Send + 'static)>>`
  = note: required because it appears within the type `error_chain::State`
  = note: required because it appears within the type `patch_rs::PatchError`
  = note: required because of the requirements on the impl of `failure::Fail` for `patch_rs::PatchError`
  = note: required because of the requirements on the impl of `std::convert::From<patch_rs::PatchError>` for `failure::Error`
  = note: required because of the requirements on the impl of `std::convert::Into<failure::Error>` for `patch_rs::PatchError`
  = note: required by `rlua::Error::external`

How about this.0.process().ok()? The downside is that we lost the error message.

juchiast commented 5 years ago

error-chain is not compatible with failure: https://github.com/rust-lang-nursery/error-chain/issues/240

I'm doing this:

            this.0
                .process()
                .map_err(|e| rlua::Error::external(failure::err_msg(e.to_string())))
dariusc93 commented 5 years ago

It looks like its related to how error-chain is designed. Honestly I havent used that myself especially since I thought error-chain crate was being deprecated in favor of the failure crate (since it wasnt being maintain to my knowledge). I went on and switch to the failure crate in the patch-rs. Could you check to make sure it is working for you as well and update it accordingly (including crate version, if deem necessary)?

juchiast commented 5 years ago

@dariusc93 Ok, I tested current master branch of patch-rs, it works! Publish a new version to crates.io then I'll update this PR.

hedgar2017 commented 5 years ago

@juchiast done, v0.3.0 is out there. @dariusc93 I agree, I should have used failure from the beginning. Some of my projects still use error-chain so for some reason I went for that.

naturallymitchell commented 5 years ago

@hedgar2017 :+1: :+1: @juchiast :+1: :+1: @dariusc93 :+1: :+1: