Add text message sending support

[yabobay]

No description provided.

[codecov]

Codecov Report

❌ Patch coverage is 0% with 30 lines in your changes missing coverage. Please review.
✅ Project coverage is 37.99%. Comparing base (ce589b6) to head (033a8d3).

Files with missing lines	Patch %	Lines
cli/src/main.rs	0.00%	30 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #325      +/-   ##
==========================================
- Coverage   38.21%   37.99%   -0.22%     
==========================================
  Files          19       19              
  Lines        3326     3353      +27     
==========================================
+ Hits         1271     1274       +3     
- Misses       2055     2079      +24     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[yabobay]

Also i figure that async-std being unmaintained and causing a cargo deny problem is okay since this project already depended on async-std

[meejah]

Hello,

Thanks for the PR!

I've not personally done any actual work in this repository, but yes this was "already" using async-std so I wouldn't imagine denying this PR based on that would be fair / a good idea :)

That said, it appears the Rust world as entirely moved on from that, so this project should as well...

[felinira]

That said, it appears the Rust world as entirely moved on from that, so this project should as well...

Yeah, migrating to smol (futures-lite/async-io) is definitely at the very top of the todo list. I'll see if i can soon find some spare cycles to burn on this, but if someone wants to do it first I wouldn't complain.

About the actual content of this PR, I will try it out soon, but it already looks very good. Thanks!

[felinira]

When I try sending a text message with the python client:

# wormhole send --text "Test"

and then receive with the same one:

# wormhole receive
[snip]
Test

I get the message inline in the terminal.

When I send a text message with this PR I instead receive a message.txt file with the message.

So this does not work as I would expect text message sending to work. I suppose the protocol implementation is slightly different.

[yabobay]

So this does not work as I would expect text message sending to work. I suppose the protocol implementation is slightly different.

Looking into it now and i'm thinking of refactoring Offer from a struct into an enum with its current content field being a variant called Files and then adding one called Text which just contains an OfferEntry<T>. Should i do so? (I haven't looked at the protocol side yet but that seems about right from checking the Python version a bit)

Edit: the other idea i had was to just make Offer use an Option<String> instead of a String in the BTreeMap but that seems very inelegant

[felinira]

I am deeply unhappy with the entire Offer API in general. However refactoring the whole thing to an enum would probably make it even more convoluted than before.

Part of why the offer API is how it is is because WASM is rather strange to support. But we can probably still do much better.

I am not sure I have an easy solution here that makes Offer less complicated instead of more. (And I certainly wouldn't expect from you to refactor the entire thing)

[felinira]

Looking at it some more, you can probably create a new OfferEntry of type Text

[yabobay]

Looking at it some more, you can probably create a new OfferEntry of type Text

But wouldn't that still be encased in Offer? I thought the String keys in the BTreeMap were for filenames, which aren't needed in this case.

OfferEntry::RegularFile as it exists now, with no filename embedded in it, seems appropriate

[felinira]

The whole thing builds up a tree. This mostly for directory offers in transfer v2 / dilation which is not yet stable.

I wonder if it's possible to root the offer tree on OfferEntry instead of on a special Offer root node. A offer with multiple files would then just be a root-node directory.

Or have Offer be a flat struct with just one OfferEntry field, if that's easier to model.

If you then put a RegularFile node in the root, this would essentially be a File with no filename -> a text message. That might more closely resemble the actual protocol as well.

[yabobay]

I wonder if it's possible to root the offer tree on OfferEntry instead of on a special Offer root node. A offer with multiple files would then just be a root-node directory.

Wouldn't it make sense then to make Offer be an enum or something that takes the form of a tree of OfferEntrys or a plain text message?

[felinira]

I would try to stay close to the original transfer v1 protocol and the proposed dilated file transfer protocol and have the following offer entries as a flat enum:

• File
• Directory
• Message

I will do some experiments along those lines and come back to you to not propose some impossible direction to you :)

[meejah]

The current WIP of dilated file transfer in Python is https://github.com/meejah/magic-wormhole/tree/445.file-transfer-v2-prototype which might provide some inspiration.

That said I havent tried text messages in there yet.

From my current (rapidly improving) Rust knowledge an Enum type as proposed above makes some sense. Ill try to look deeper at the Rust code in the coming days and produce a better opinion

[meejah]

The v1 protocol is unlikely to change at all (if at all possible) and I feel that the Dilated File Transfer is the right shape .. But some tweaks are always possible (and now before there's anything to be backwards compatible with is definitely the easiest time to make changes).

[felinira]

I would try to stay close to the original transfer v1 protocol and the proposed dilated file transfer protocol and have the following offer entries as a flat enum:

* File

* Directory

* Message

I will do some experiments along those lines and come back to you to not propose some impossible direction to you :)

I tried a few things and there is an issue with this proposal:

You can't nest text messages in a directory. And ideally this shouldn't be allowed at the type level either. So maybe I have come full circle and an enum at the top level is the right choice unless you want to redo the entire rust API.

[yabobay]

You can't nest text messages in a directory. And ideally this shouldn't be allowed at the type level either. So maybe I have come full circle and an enum at the top level is the right choice unless you want to redo the entire rust API.

Around the time you posted this i tried to make this change and went through all the compiler errors, but alas Rust isn't my language and i got stuck on one final one, so i've pushed the code as-is to branch enum on my fork. You can take a look at it if you want.