Opened 11 years ago

Closed 8 years ago

Last modified 8 years ago

#4526 closed patch (fixed)

Jabber /join-command

Reported by: Solarius Owned by: deryni
Milestone: 2.7.11 Component: XMPP
Version: 2.3.1 Keywords:
Cc: mcepl, wyuka

Description

Hi! This "patch" will modify the /join command in jabber chat room following way: Normaly, when user types "/join aroom", user will join to room "aroom" in current server (as usual), but if user writes /join aroom@… user will join room "aroom" at server "conference.jabber.org"

Here is the patch for libpurple/protocols/jabber/jabber.c:

2137,2139d2136
<       size_t str_find;
<       int str_len;
<       char roomname[255], servername[255];
2146,2162c2143,2144
< 
<       str_find = strcspn (args[0], "@");
<       str_len = strlen (args[0]);
<       memset (roomname, 0x00, 255);
<       memset (servername, 0x00, 255);
< 
<       if (str_find!=str_len && str_find < 255 && (str_len-str_find) < 255) {
<               strncpy (roomname,args[0], str_find);
<               strncpy (servername,args[0]+(str_find+1), str_len-str_find);
< 
<               g_hash_table_replace(components, "room", roomname);
<               g_hash_table_replace(components, "server", servername);
<       } else {
<               g_hash_table_replace(components, "room", args[0]);
<               g_hash_table_replace(components, "server", chat->server);
<       }
< 
---
>       g_hash_table_replace(components, "room", args[0]);
>       g_hash_table_replace(components, "server", chat->server);

I've also attached the modified file also, just in case.

And yep, the patch was against 2.3.1 tree ;)

Attachments (3)

jabber.c (72.9 KB) - added by Solarius 11 years ago.
pidgin-extended-xmpp-join.patch (2.1 KB) - added by mcepl 10 years ago.
Corrected version of patch
xmpp-join.patch (1.6 KB) - added by wyuka 8 years ago.

Download all attachments as: .zip

Change History (26)

Changed 11 years ago by Solarius

comment:1 Changed 11 years ago by rekkanoryo

  • pending changed from 0 to 1

Please use the "diff -udp" to generate patches.

comment:2 Changed 11 years ago by Solarius

  • pending changed from 1 to 0

OK, here it goes:

--- libpurple/protocols/jabber/jabber.c 2007-12-31 00:15:09.000000000 +0200
+++ /home/ville/sorsat/pidgin-2.3.1/libpurple/protocols/jabber/jabber.c 2007-12-07 16:37:06.000000000 +0200
@@ -2134,32 +2134,14 @@ static PurpleCmdRet jabber_cmd_chat_join
 {
        JabberChat *chat = jabber_chat_find_by_conv(conv);
        GHashTable *components;
-       size_t str_find;
-       int str_len;
-       char roomname[255], servername[255];
 
        if(!chat || !args || !args[0])
                return PURPLE_CMD_RET_FAILED;
 
        components = g_hash_table_new_full(g_str_hash, g_str_equal, NULL, NULL);
 
-
-       str_find = strcspn (args[0], "@");
-       str_len = strlen (args[0]);
-       memset (roomname, 0x00, 255);
-       memset (servername, 0x00, 255);
-
-       if (str_find!=str_len && str_find < 255 && (str_len-str_find) < 255) {
-               strncpy (roomname,args[0], str_find);
-               strncpy (servername,args[0]+(str_find+1), str_len-str_find);
-
-               g_hash_table_replace(components, "room", roomname);
-               g_hash_table_replace(components, "server", servername);
-       } else {
-               g_hash_table_replace(components, "room", args[0]);
-               g_hash_table_replace(components, "server", chat->server);
-       }
-
+       g_hash_table_replace(components, "room", args[0]);
+       g_hash_table_replace(components, "server", chat->server);
        g_hash_table_replace(components, "handle", chat->handle);
        if(args[1])
                g_hash_table_replace(components, "password", args[1]);

comment:3 Changed 11 years ago by rekkanoryo

  • Component changed from unclassified to XMPP
  • Owner changed from lschiere to nwalp

comment:4 Changed 11 years ago by Solarius

...and because the description text needs modifications also, this would be the better patch:

--- libpurple/protocols/jabber/jabber.c 2007-12-31 01:11:46.000000000 +0200
+++ /home/ville/sorsat/pidgin-2.3.1/libpurple/protocols/jabber/jabber.c 2007-12-07 16:37:06.000000000 +0200
@@ -2134,32 +2134,14 @@ static PurpleCmdRet jabber_cmd_chat_join
 {
        JabberChat *chat = jabber_chat_find_by_conv(conv);
        GHashTable *components;
-       size_t str_find;
-       int str_len;
-       char roomname[255], servername[255];
 
        if(!chat || !args || !args[0])
                return PURPLE_CMD_RET_FAILED;
 
        components = g_hash_table_new_full(g_str_hash, g_str_equal, NULL, NULL);
 
-
-       str_find = strcspn (args[0], "@");
-       str_len = strlen (args[0]);
-       memset (roomname, 0x00, 255);
-       memset (servername, 0x00, 255);
-
-       if (str_find!=str_len && str_find < 255 && (str_len-str_find) < 255) {
-               strncpy (roomname,args[0], str_find);
-               strncpy (servername,args[0]+(str_find+1), str_len-str_find);
-
-               g_hash_table_replace(components, "room", roomname);
-               g_hash_table_replace(components, "server", servername);
-       } else {
-               g_hash_table_replace(components, "room", args[0]);
-               g_hash_table_replace(components, "server", chat->server);
-       }
-
+       g_hash_table_replace(components, "room", args[0]);
+       g_hash_table_replace(components, "server", chat->server);
        g_hash_table_replace(components, "handle", chat->handle);
        if(args[1])
                g_hash_table_replace(components, "password", args[1]);
@@ -2375,7 +2357,7 @@ void jabber_register_commands(void)
                          PURPLE_CMD_FLAG_CHAT | PURPLE_CMD_FLAG_PRPL_ONLY |
                          PURPLE_CMD_FLAG_ALLOW_WRONG_ARGS, "prpl-jabber",
                          jabber_cmd_chat_join,
-                         _("join: &lt;room[@server]&gt; [password]:  Join a chat on this server."),
+                         _("join: &lt;room&gt; [server]:  Join a chat on this server."),
                          NULL);
        purple_cmd_register("kick", "ws", PURPLE_CMD_P_PRPL,
                          PURPLE_CMD_FLAG_CHAT | PURPLE_CMD_FLAG_PRPL_ONLY |

comment:5 follow-up: Changed 11 years ago by QuLogic

Firstly, I think your diff is backwards.

Second, that splitting stuff looks a bit complicated. Couldn't you just use g_strsplit instead?

comment:6 Changed 11 years ago by datallah

  • Type changed from enhancement to patch

comment:7 Changed 11 years ago by deryni

  • Owner changed from nwalp to deryni

Changed 10 years ago by mcepl

Corrected version of patch

comment:8 Changed 10 years ago by mcepl

Which the above version of the patch, it cleanly builds on the top of Fedora's pidgin 2.5.3, and I have to confirm it works pretty well.

comment:9 Changed 10 years ago by mcepl

s/Which/With/?

sorry

comment:10 Changed 10 years ago by rekkanoryo

  • Milestone set to Patches Needing Review

comment:11 in reply to: ↑ 5 ; follow-up: Changed 10 years ago by QuLogic

  • Milestone changed from Patches Needing Review to Patches Needing Improvement

This question is still not answered:

Second, that splitting stuff looks a bit complicated. Couldn't you just use g_strsplit instead?

comment:12 in reply to: ↑ 11 Changed 10 years ago by mcepl

Replying to QuLogic:

Couldn't you just use g_strsplit instead?

Certainly, I have no clue about programming with glib :)

Changed 8 years ago by wyuka

comment:13 Changed 8 years ago by wyuka

I used g_strsplit to replace the splitting code. Should be ok now, i guess.

comment:14 follow-up: Changed 8 years ago by darkrain42

  • Milestone changed from Patches Needing Improvement to 2.7.10

This looks pretty good, though I haven't tested.

It looks like this won't allow joining a room while specifying a resource (a.k.a. in-room nick), a la "/join devel@…/darkrain42@…"). (jabber_id_new might be a good use here -- otherwise, is fine, and it's not really a 'regression')

comment:15 in reply to: ↑ 14 Changed 8 years ago by mcepl

Replying to darkrain42:

It looks like this won't allow joining a room while specifying a resource (a.k.a. in-room nick), a la "/join devel@…/darkrain42@…"). (jabber_id_new might be a good use here -- otherwise, is fine, and it's not really a 'regression')

Well, with all due respect your example looks weird ... using /join when you actually want /query (to use IRC terminology) doesn't make me happy. Sure, it is not related to this patch, just that libpurple seems to keep this distinction very clear so why to muddle it here?

comment:16 follow-up: Changed 8 years ago by deryni

/join room@server/nick@host isn't an IRC query. It is a request to join a room with a specific room nickname instead of a default room nickname (a concept IRC doesn't have) and yes it does look weird you can consider the much more normal looking "/join devel@conference.pidgin.im/some_user_name" if that makes it easier to see/understand. And yeah, I'd imagine jabber_id_new is likely the right sort of thing to be using here.

comment:17 in reply to: ↑ 16 Changed 8 years ago by mcepl

Replying to deryni:

It is a request to join a room with a specific room nickname instead of a default room nickname

Damn, yes, you're right. I thought about direct communication with other nick in a random MUC.

OTOH, just because of inevitable confusion it may cause, I would probably stand against this "feature" anyway ;)).

comment:18 Changed 8 years ago by deryni

I'm not sure it'd cause confusion but I'm also not sure I'd encourage it to be documented in the /help output either. (I'd probably have the help output say something like "<room jid>" to include the possibility for people who know about it but without cluttering it for people who don't, possibly even with an example "/join devel@…" or similar in the output.)

But more specifically I think this patch should use jabber_id_new so that /join r@s/n doesn't cause it to attempt to join room 'r' on server 's/n', whether we use or discard the resource part is a lesser issue than that (though once we have it I'm not sure not using it makes sense).

comment:19 follow-up: Changed 8 years ago by darkrain42

Solarius, wyuka: What names (and email addresses) should be used for credit/COPYRIGHT?

comment:20 Changed 8 years ago by darkrain42@…

  • Resolution set to fixed
  • Status changed from new to closed

(In dfcd7327f9dca20e01e193aa8ac416625e69c3d0):
jabber: Add support for joining a room based not on the current server.

An extension of a patch from Solarius, mcepl, and wyuka; I added in jabber_id_new parsing. Fixes #4526. I'll tweak the context help after the string freeze.

comment:21 in reply to: ↑ 19 ; follow-up: Changed 8 years ago by wyuka

Replying to darkrain42:

Solarius, wyuka: What names (and email addresses) should be used for credit/COPYRIGHT?

Tirtha 'wyuka' Chatterjee <tirtha.p.chatterjee@…>

comment:22 in reply to: ↑ 21 Changed 8 years ago by wyuka

Replying to wyuka:

Replying to darkrain42:

Solarius, wyuka: What names (and email addresses) should be used for credit/COPYRIGHT?

Tirtha 'wyuka' Chatterjee <tirtha.p.chatterjee@…>

Tirtha 'wyuka' Chatterjee <tirtha.p.chatterjee AT gmail DOT com>

comment:23 Changed 8 years ago by darkrain42@…

(In c0b910c136c2999321b3ee2969b3e2c60abd11b8):
Credit wyuka, whose apparent goal here is to make me linewrap things. Refs #4526.

Note: See TracTickets for help on using tickets.
All information, including names and email addresses, entered onto this website or sent to mailing lists affiliated with this website will be public. Do not post confidential information, especially passwords!