Closed Bug 954231 Opened 10 years ago Closed 10 years ago

Add help strings for Instantbird commands

Categories

(Chat Core :: General, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: clokep, Assigned: clokep)

References

Details

(Whiteboard: [0.3-wanted-beta])

Attachments

(2 files, 2 obsolete files)

*** Original post on bio 797 at 2011-05-23 14:14:00 UTC ***

The libpurple commands all have help strings but we never added any for our commands (raw, help and the status updating [1]). These commands should have localized help strings added (especially since you can view these easily in the UI from bug 954126 (bio 691)).

[1] http://lxr.instantbird.org/instantbird/source/instantbird/modules/ibCore.jsm#98
Depends on: 954126
*** Original post on bio 797 at 2011-05-23 14:17:11 UTC ***

I would suggest moving the status commands to near the other 2 for simplicity.
Whiteboard: [0.3-wanted-beta]
Attached patch v1.0 (obsolete) — Splinter Review
*** Original post on bio 797 as attmnt 646 at 2011-05-24 01:51:00 UTC ***

Help strings for raw, help and the status commands. Let me know if these strings make sense flo (in particular to translators, I'm not sure if extra notes are needed).
Attachment #8352389 - Flags: review?(florian)
Attached patch v1.1 (obsolete) — Splinter Review
*** Original post on bio 797 as attmnt 647 at 2011-05-24 02:04:00 UTC ***

I messed up (of course), last patch had a debug statement in it, this one removes those two or three lines.
Attachment #8352390 - Flags: review?(florian)
Comment on attachment 8352389 [details] [diff] [review]
v1.0

*** Original change on bio 797 attmnt 646 at 2011-05-24 02:04:28 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8352389 - Attachment is obsolete: true
Attachment #8352389 - Flags: review?(florian)
Comment on attachment 8352390 [details] [diff] [review]
v1.1

*** Original change on bio 797 attmnt 647 at 2011-05-24 07:07:30 UTC ***

>diff --git a/purple/locales/en-US/commands.properties b/purple/locales/en-US/commands.properties
>--- a/purple/locales/en-US/commands.properties
>+++ b/purple/locales/en-US/commands.properties
>@@ -5,3 +5,15 @@
> #  %S is the command name the user typed.
> noCommand=No '%S' command.
> noHelp=No help message for the '%S' command, sorry!
>+
>+# LOCALIZATION NOTE (command help strings)
>+rawHelpString=raw <message>: sends a raw command to the server.

This message looks a lot like the quote command on the IRC prpl ;).
From what I remember, the raw command was added to send a message into a conversation without the HTML entities being escaped.

>+# LOCALIZATION NOTE (status help strings):

# LOCALIZATION NOTE (statusCommand):
#  The first %S is replaced with a status command name
#   (one of "back", "away", "busy", "dnd", or "offline").
#  The second %S is replaced with the localized version of that status type
#   (one of the 5 strings below).

I'm sad that the offline command takes a status message parameter, but that's what the code does :-/.

>diff --git a/purple/purplexpcom/src/imCommands.js b/purple/purplexpcom/src/imCommands.js
>--- a/purple/purplexpcom/src/imCommands.js
>+++ b/purple/purplexpcom/src/imCommands.js
>@@ -56,6 +56,7 @@
>     this._commands = {};
>     this.registerCommand({
>       name: "raw",
>+      helpString: bundle.GetStringFromName("rawHelpString"),

        get helpString() bundle.GetStringFromName("rawHelpString"),

This way the bundle getter won't be executed when registering the commands (= at startup) but if the user ever uses the help command to see that message.
Of course, this change is needed for the other commands too for it to be of any value.
Attachment #8352390 - Flags: review?(florian) → review-
*** Original post on bio 797 at 2011-05-24 07:09:34 UTC ***

Comment on attachment 8352390 [details] [diff] [review] (bio-attmnt 647)
v1.1

I forgot to say in the previous comment that it looks good otherwise! :)
Attached patch v1.2Splinter Review
*** Original post on bio 797 as attmnt 653 at 2011-05-24 22:41:00 UTC ***

This should meet all the comments! :)
Attachment #8352396 - Flags: review?(florian)
Comment on attachment 8352390 [details] [diff] [review]
v1.1

*** Original change on bio 797 attmnt 647 at 2011-05-24 22:41:34 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8352390 - Attachment is obsolete: true
Summary: Add help string for Instantbird commands → Add help strings for Instantbird commands
Comment on attachment 8352396 [details] [diff] [review]
v1.2

*** Original change on bio 797 attmnt 653 at 2011-05-25 16:27:41 UTC ***

r=me with additional changes discussed on IRC.
Attachment #8352396 - Flags: review?(florian) → review+
*** Original post on bio 797 as attmnt 654 at 2011-05-25 16:28:00 UTC ***

From the IRC log: 16:16:24 <flo> the |cmd| variable is not usable inside the getter, as it's not a copy of the variable at the time the getter was created, but a reference to the variable, whose value changes at each iteration. Your patch without modification displays the help message of the "offline" command for all the status commands.
*** Original post on bio 797 at 2011-05-25 16:57:39 UTC ***

Attachment 8352396 [details] [diff] (bio-attmnt 653) and 654: https://hg.instantbird.org/instantbird/rev/b13df4582161
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → 0.3b1
You need to log in before you can comment on or make changes to this bug.