ayrat555 / frankenstein

Telegram bot API client for Rust
Do What The F*ck You Want To Public License
268 stars 28 forks source link

try fit wasm partially #225

Open xlfish233 opened 3 weeks ago

xlfish233 commented 3 weeks ago

Add the wasm feature, ensuring it functions correctly with worker-rs. Temporarily remove the logic related to request_with_form_data involving files; I will implement a replacement for this later. Note that multipart may not function with the wasm feature; I will look for an alternative solution in the future.

224

xlfish233 commented 3 weeks ago

not(target_arch = "wasm32") test pass on my machine.

xlfish233 commented 3 weeks ago

Temporarily remove the logic related to request_with_form_data involving files; I will implement a replacement for this later.暂时删除涉及文件的request_with_form_data相关逻辑;我稍后将对此进行替换。

This comment raised some questions but I think it only targets wasm and is not affecting the existing features for unix/windows targets?这个评论提出了一些问题,但我认为它只针对 wasm,不会影响 unix/windows 目标的现有功能?

I have not looked into this that much so far, only glanced over it to give an initial feedback. I hope it doesn't feel too rushed.到目前为止,我还没有对此进行太多研究,只是浏览了一下以给出初步反馈。我希望它不会让人感觉太匆忙。

Thank you for reviewing my work. Here are some details regarding this commit:

This commit was somewhat rushed as it is intended for my bot project, which required a quick turnaround.

All modifications are focused on the WebAssembly (WASM) target and its features. The logic for other target platforms, such as Unix and Windows, remains unchanged, so I believe the functionality should still be intact on these systems.

I will carefully consider your suggestions and work on developing an alternative method for attaching files in the request_with_form_data function.

EdJoPaTo commented 3 weeks ago

Personally I am fine with a PR not including file support for WASM as long as it stays intact for existing users. Having WASM without file support is more than having no WASM support ;)

xlfish233 commented 3 weeks ago

I've just updated some of the work by incorporating most of your suggestions. However, I believe there's still significant room for improvement. Could you provide further feedback?

xlfish233 commented 3 weeks ago

Thank you for your valuable feedback on my pull request. I appreciate the insights and suggestions you've provided, many of which I hadn't considered and are definitely worth adopting. Due to my current involvement in other projects, it might take some time before I can continue with the next set of changes for this PR. I'll keep you updated on my progress. @EdJoPaTo

EdJoPaTo commented 3 weeks ago

@xlfish233 I should be able to push to your branch (Edits by maintainers are allowed). I can remove the second feature and fully do it via cfg(target_arch).

For other things I would like your input and wait for you when you have the time.

Thank you for your valuable feedback on my pull request. I appreciate the insights and suggestions you've provided, many of which I hadn't considered and are definitely worth adopting.

Likewise! Working together on this seems working well to get this integrated!

xlfish233 commented 2 weeks ago

most recent commit i try only use target_arch = "wasm32" to difference it. now,it work's well at existing and can run with --target wasm32-unknown-unknown --no-default-features --features async-http-client