Add XEP-0054 vCard support #1757

techmetx11 · 2022-09-24T22:14:13Z

Only nicknames, photos, birthdays, addresses, telephone numbers, emails,
JIDs, titles, roles, notes, and URLs are supported

jubalh · 2022-09-25T08:11:27Z

@mdosch can you test whether it behaves like you imanged when you opened the issue?

mdosch · 2022-09-26T17:47:49Z

I tried out requesting vcards and this are my findings:

/vcard get does autocompletion through occupants in a MUC but not through roster in the main window
/vcard get in a 1-1 chat shows my own vcard, not the one of the contact

techmetx11 · 2022-09-26T17:55:59Z

I tried out requesting vcards and this are my findings:

* `/vcard get` does autocompletion through occupants in a MUC but not through roster in the main window

* `/vcard get` in a 1-1 chat shows my own vcard, not the one of the contact

Thanks

src/command/cmd_ac.c

jubalh · 2022-09-29T08:15:32Z

src/command/cmd_defs.c

+              "/vcard refresh",
+              "/vcard save")
+      CMD_DESC(
+              "Read your vCard or a user's vCard, get a user's avatar via their vCard, or modify your vCard. If no arguments are given, your vCard will be displayed in a new window, or an existing vCard window.")


Does it make sense to change Read your vCard (XEP-XXXX) so that the experienced user knows which vcard XEP is used?
I didn't read the corresponding XEPs yet.
Not sure which one is more up to date and whether later we will just leave the same command but use another XEP in the background.

@mdosch any opinion on this?

@mdosch ping

src/config/files.h

src/xmpp/vcard.h

jubalh

First review looks good! Thanks for your work!
I added some comments/questions.

Also:
Did you test the new functionality with valgrind to check for any leaks? It's described here: https://github.com/profanity-im/profanity/blob/master/CONTRIBUTING.md
If you need any help about that, please let me know.

techmetx11 · 2022-09-29T17:38:09Z

First review looks good! Thanks for your work! I added some comments/questions.

Also: Did you test the new functionality with valgrind to check for any leaks? It's described here: https://github.com/profanity-im/profanity/blob/master/CONTRIBUTING.md If you need any help about that, please let me know.

Yes, I have. Thankfully the only memory leaks I found were with the vcardwin get string/title function

techmetx11 · 2022-10-13T10:38:09Z

src/command/cmd_funcs.c

+cmd_vcard_photo(ProfWin* window, const char* const command, gchar** args)
+{
+    char* operation = args[1];
+    char* user = args[2];


How do we design the command to be able to get the user's own avatar
Commands like /vcard photo open may work but /vcard photo open 4 (as in index) may conflict with the nick/contact parameter
Or /vcard photo save but /vcard photo save filename test.png or /vcard photo save index 4 may lead the command into thinking that it's supposed to get an avatar from a user nicknamed filename or index

I'm missing some context.
What exactly is /vcard photo open 4 supposed to do?

I'm missing some context. What exactly is /vcard photo open 4 supposed to do?

Download the avatar from a user nicknamed 4. In some cases, this might get confused with downloading the 4th avatar from your vCard

But is there a way (or should there be a way) to download multiple avatars? If yes we could just have another parameter /vcard photo open-index 4. We have problems like this with other commands also we would need to check how we solved things there. Either by having a new (sub)command, a command with - or reorganizing some commands.

Or maybe we could have /vcard photo open-self [<index>] and /vcard photo save-self [filename <filename>] [index <index>]

But is there a way (or should there be a way) to download multiple avatars? If yes we could just have another parameter /vcard photo open-index 4. We have problems like this with other commands also we would need to check how we solved things there. Either by having a new (sub)command, a command with - or reorganizing some commands.

Yes, there is

Or maybe we could have /vcard photo open-self [] and /vcard photo save-self [filename ] [index ]

Sounds good. And for other users?

Or maybe we could have /vcard photo open-self [] and /vcard photo save-self [filename ] [index ]

Sounds good. And for other users?

/vcard photo open <nick/contact> [<index>]
/vcard photo save <nick/contact> [filename <filename>] [index <index>]

Sounds good to me.

techmetx11 · 2022-10-14T20:00:19Z

Weird, it compiled fine for me

jubalh · 2022-10-14T20:01:47Z

 src/xmpp/vcard.c: In function '_vcard_photo_result':
src/xmpp/vcard.c:1374:18: error: too many arguments to function 'call_external'

techmetx11 · 2022-10-14T20:03:13Z

 src/xmpp/vcard.c: In function '_vcard_photo_result':
src/xmpp/vcard.c:1374:18: error: too many arguments to function 'call_external'

Oh, I forgot
Sorry my bad :3

jubalh · 2022-10-14T21:54:44Z

Great!
I'll try to review soon.
Please read this and this too. After this you can squash your commits together in a way that makes sense.

jubalh · 2022-10-18T12:50:46Z

AFAIK @sjaeckel plans to improve some parts of this PR with @techmetx11, so setting review on him.
I still think in the meantime you could squash the commits together (and rebase on latest master to remove the conflict in cmd_ac.c) @techmetx11 :)

Only nicknames, photos, birthdays, addresses, telephone numbers, emails, JIDs, titles, roles, notes, and URLs are supported Due to the synopsis array not having enough space, `/vcard photo open-self` and `/vcard photo save-self` are not documented properly in the synopsis section of the `/vcard` command, but they are documented in the arguments section Fixed memory leak in vcard autocomplete (thanks to debXwoody)

sjaeckel · 2022-10-19T06:01:56Z

AFAIK @sjaeckel plans to improve some parts of this PR with @techmetx11, so setting review on him.

I don't have the time for a deeper review and I don't want to block this, so I removed myself from the reviewer list.

jubalh · 2022-10-20T13:45:28Z

Thanks for your contribution @techmetx11

@techmetx11

Implemented for next release in #1757 by @techmetx11.

techmetx11 mentioned this pull request Sep 24, 2022

Add XEP-0054 vCard support #1751

Closed

jubalh self-requested a review September 25, 2022 08:10

jubalh assigned techmetx11 Sep 25, 2022

jubalh added the feature label Sep 25, 2022

techmetx11 force-pushed the feature/1158-vcard branch 2 times, most recently from c7dc068 to e338da5 Compare September 26, 2022 18:27

jubalh added this to the 0.14.0 milestone Sep 27, 2022

jubalh reviewed Sep 29, 2022

View reviewed changes

src/command/cmd_ac.c Show resolved Hide resolved

jubalh reviewed Sep 29, 2022

View reviewed changes

src/config/files.h Show resolved Hide resolved

jubalh reviewed Sep 29, 2022

View reviewed changes

src/xmpp/vcard.h Outdated Show resolved Hide resolved

jubalh reviewed Sep 29, 2022

View reviewed changes

techmetx11 commented Oct 13, 2022

View reviewed changes

techmetx11 force-pushed the feature/1158-vcard branch 2 times, most recently from 0bd5a66 to 3329430 Compare October 14, 2022 20:31

jubalh requested a review from sjaeckel October 18, 2022 12:50

techmetx11 force-pushed the feature/1158-vcard branch 3 times, most recently from 15a496a to a445ee9 Compare October 18, 2022 22:18

techmetx11 force-pushed the feature/1158-vcard branch from a445ee9 to f934c5b Compare October 18, 2022 22:24

sjaeckel removed their request for review October 19, 2022 05:58

jubalh merged commit 980fc18 into profanity-im:master Oct 20, 2022

jubalh modified the milestones: 0.14.0, next Oct 20, 2022

jubalh added a commit that referenced this pull request Jul 3, 2023

Add xep-0054 info to doap file

9c7a27a

Implemented for next release in #1757 by @techmetx11.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add XEP-0054 vCard support #1757

Add XEP-0054 vCard support #1757

techmetx11 commented Sep 24, 2022

jubalh commented Sep 25, 2022

mdosch commented Sep 26, 2022

techmetx11 commented Sep 26, 2022

jubalh Sep 29, 2022

jubalh Oct 13, 2022

jubalh left a comment

techmetx11 commented Sep 29, 2022

techmetx11 Oct 13, 2022

jubalh Oct 13, 2022

techmetx11 Oct 13, 2022 •

edited

Loading

jubalh Oct 13, 2022

techmetx11 Oct 13, 2022 •

edited

Loading

jubalh Oct 13, 2022

techmetx11 Oct 13, 2022

jubalh Oct 14, 2022

techmetx11 commented Oct 14, 2022

jubalh commented Oct 14, 2022

techmetx11 commented Oct 14, 2022

jubalh commented Oct 14, 2022

jubalh commented Oct 18, 2022 •

edited

Loading

sjaeckel commented Oct 19, 2022

jubalh commented Oct 20, 2022

Add XEP-0054 vCard support #1757

Add XEP-0054 vCard support #1757

Conversation

techmetx11 commented Sep 24, 2022

jubalh commented Sep 25, 2022

mdosch commented Sep 26, 2022

techmetx11 commented Sep 26, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jubalh left a comment

Choose a reason for hiding this comment

techmetx11 commented Sep 29, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

techmetx11 Oct 13, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

techmetx11 Oct 13, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

techmetx11 commented Oct 14, 2022

jubalh commented Oct 14, 2022

techmetx11 commented Oct 14, 2022

jubalh commented Oct 14, 2022

jubalh commented Oct 18, 2022 • edited Loading

sjaeckel commented Oct 19, 2022

jubalh commented Oct 20, 2022

techmetx11 Oct 13, 2022 •

edited

Loading

techmetx11 Oct 13, 2022 •

edited

Loading

jubalh commented Oct 18, 2022 •

edited

Loading