feat: Add import-vcard and make-vcard commands to repl #7048
Conversation
- Added import-vcard and make-vcard commands to deltachat-repl. These wrap the corresponding import_vcard() and make_vcard() operations from src/contact.rs. Each command takes a file path argument specifying the location of the vCard file to be read or written. The make-vcard command takes one or more IDs to write to the file. - Documented commands in "Using the CLI client" section of README.md. closes chatmail#7047
README.md
Outdated
|
|
||
| ``` | ||
| > addcontact [email protected] | ||
| > import-vcard keyfriend.vcard |
There was a problem hiding this comment.
Maybe call it "key-contact.vcard" because "key-contact" is an agreed term in the project. This way the documentation would read clearer
There was a problem hiding this comment.
Thanks! Fixed as suggested in latest revision.
deltachat-repl/src/cmdline.rs
Outdated
| ensure!(!arg1.is_empty(), "Argument <file> missing."); | ||
| let vcard_content = fs::read_to_string(&arg1.to_string()).await?; | ||
| let contacts = import_vcard(&context, &vcard_content).await?; | ||
| println!("vCard contacts imported to:"); |
There was a problem hiding this comment.
What follows is a list of contact objects and their IDs, so this is saying the content of the vCard file was imported to that list of contacts. Here is an example of what it currently looks like:
vCard contacts imported to:
Contact#Contact#10: [email protected] <[email protected]>
Contact#Contact#11: [email protected] <[email protected]>
Contact#Contact#12: [email protected] <[email protected]>
I suppose it also works fine without the "to", so I'll just remove it.
deltachat-repl/src/cmdline.rs
Outdated
| ensure!(!arg1.is_empty(), "Argument <file> missing."); | ||
| ensure!(!arg2.is_empty(), "Argument <contact-id> missing."); | ||
| let mut contact_ids = vec![]; | ||
| for x in arg2.split(' ') { |
There was a problem hiding this comment.
Consider using split_whitespace()/split_ascii_whitespace()
There was a problem hiding this comment.
Thank you! I'm new to Rust and don't know the standard library very well yet. New revision coming up soon!
- In key contact example, use project terminology "key-contact". - Removed unnecessary "to" from import-vcard output. - Use split_whitespace() to separate IDs in make-vcard. - All changes suggested in review by iequidoo.
| @@ -1218,6 +1220,24 @@ pub async fn cmdline(context: Context, line: &str, chat_id: &mut ChatId) -> Resu | |||
| log_contactlist(&context, &contacts).await?; | |||
| println!("{} blocked contacts.", contacts.len()); | |||
| } | |||
| "import-vcard" => { | |||
| ensure!(!arg1.is_empty(), "Argument <file> missing."); | |||
There was a problem hiding this comment.
Would be nice if we support whitespaces in arguments (but only if we want the repl tool to be really useful, not just for testing), AFAIU currently you can't import a vcard from, say, "Bob Smith.vcf". Maybe backslash escaping is enough
There was a problem hiding this comment.
Ah, yes, the classic filenames-with-spaces problem. This code is downstream from the logic that parses arg1 and arg2, so it would be messy to fix this without changing the whole parser strategy. (I suppose we could re-parse all of line inside the import-vcard handler, but that doesn't seem like a good path to start going down.) For the export (make-vcard), I had originally implemented this with the file as the second argument, which probably would support spaces, because of how arg2 is created, but I moved the filename to the first argument because I wanted to take advantage of the multiple contact support of make_vcard() instead of artificially limiting it to one, thus not fully exposing the interface of the underlying function. But we still would have the problem on import-vcard unless we added some other placeholder argument that forces the filename to arg2.
Other commands in the repl have the same limitation, like sendhtml, so without major changes, we probably just have to live with only handling "nice" filenames. (And if it ever is revised, then we hope nobody complains that we broke the syntax for sending a file whose name is a single backslash, the "xkcd 1172" effect.)
There was a problem hiding this comment.
Yes, of course supporting spaces in filenames should be implemented for all commands then, so better not in this PR. Breaking support for "\" as a filename is fine, in favor of "\\". For now let's wait a bit for other reviewers, if any, and merge this.
closes #7047