Feat/completion

[a-moreira]

closes #147

[a-moreira]

this implementation is not ideal because the trait Completion in dialoguer expects only one completion (a Option<String>, while in our cause it would be better to expect a vector with possible completions. I'm open to other approaches in the long run, maybe using the crate inquire instead of dialoguer in the repo, which also provides Autocompletion. What do you think? @piegamesde

This implementation should be good enough though until this decision is made.

[a-moreira]

btw also ran clippy in other parts of the repo, hope it's not a problem.

[a-moreira]

some tests still breaking, will fix soon.

@piegamesde maybe it's not worth it to change to dialoguer, then we would need a way to expose the API you had written in a "loop" to get the input.

[piegamesde]

I think dialoguer is fine for now. It only does completion and not suggestions, meaning that if there are multiple matching values no completion should be performed. This is not optimal, but okay for now (the Python implementation does it like this as well).

However, the magic wormhole library should not depend on dialoguer, that dependency should be exclusive to the CLI. Therefore you need to move the respective code out.

[a-moreira]

@piegamesde I'm getting a cyclic dependency error when trying to import the CLI code into the root crate to get the wordlist mod, which is necessary in src/core.rs. Do we need to have the CLI as a separate crate in the root? Maybe we can change the project structure a little bit to have the CLI as a mod inside the crate root or another approach.

[piegamesde]

CLI is separate from the rest on purpose, and if you are getting a cyclic dependency error that indicates you are doing something wrong. The library crate should never depend on the CLI or CLI code, and you trying to import CLI code in the library is what causes your error.

[a-moreira]

@piegamesde you are right. I changed to an approach of wrapping the main wordlist struct in a new type so I can implement the dialoguer traits inside the CLI, thus splitting the code for each crate according to its necessary functionalities. Also removed a couple of tests that don't make sense anymore now that it doesn't return a vector of suggestions.

[codecov]

Codecov Report

Attention: Patch coverage is 57.57576% with 28 lines in your changes missing coverage. Please review.

Project coverage is 40.50%. Comparing base (52a12e2) to head (ab35282).
Report is 92 commits behind head on main.

Files with missing lines	Patch %	Lines
cli/src/main.rs	50.00%	21 Missing ⚠️
src/transit/crypto.rs	0.00%	4 Missing ⚠️
cli/src/util.rs	0.00%	1 Missing ⚠️
src/core/wordlist.rs	88.88%	1 Missing ⚠️
src/transfer/cancel.rs	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #202      +/-   ##
==========================================
- Coverage   40.52%   40.50%   -0.03%     
==========================================
  Files          23       23              
  Lines        3326     3343      +17     
==========================================
+ Hits         1348     1354       +6     
- Misses       1978     1989      +11     

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

[a-moreira]

one cargo test action seems to be failing for some reason, but the others not. Locally it runs ok.

[piegamesde]

This method should be kept, and the wrapper for Dialoguer in the CLI should make use of it.

[piegamesde]

This function is really weird. At the very least it should have a descriptive name and documentation to tell what it does, but honestly I think it probably shouldn't exist / not be public in the first place.

[piegamesde]

This looks like a re-implementation of load_pgpwords. Why not make use of that?

[piegamesde]

I'd consider that out of scope for now.

[felinira]

Closing due to inactivity. This would need an extensive rebase either way. Please discuss the implementation in #147 if you are still interested in this feature.