789853 - Improper brackets for /leave params

Status: VERIFIED FIXED

(Other Applications :: ChatZilla, defect)

Description

[Donald Sarratt]

Attachment: 1634.patch

User Agent: Mozilla/5.0 (Windows NT 6.1; rv:14.0) Gecko/20100101 Firefox/14.0.1
Build ID: 20120713134347

Steps to reproduce:

The params for the /leave command are listed as
[USAGE]	leave [<channel-name> [<reason>]]

However it is possible to specify a reason without first specifying a channel-name, so the proper listing should be
[USAGE]	leave [<channel-name>] [<reason>]

Patch attached, it's just a single line.

Comment 1

[Donald Sarratt]

Attachment: Fix param brackets

It seems you can't upload a patch with the initial bug filing.

Comment 2

[Tony Mechelynck [:tonymec]]

Comment on attachment 659621 [details] [diff] [review]
1634.patch

In reply to comment #1:
You can tell Bugzilla later on that it is a patch. Just be sure to tick the appropriate checkbox. But no problem, you found another solution. ;-)

Comment 3

[Tony Mechelynck [:tonymec]]

P.S. That patch looks OK to me, but in order to make it part of the code, it needs a review from a ChatZilla owner or peer. I suggest that you ask one from James Ross ("Silver") who already reviewed your related code fix for bug 682682.

Comment 4

[Donald Sarratt]

No, I did not just request a review on the wrong file.

<.<

Comment 5

[James Ross]

Comment on attachment 659622 [details] [diff] [review]
Fix param brackets

Review of attachment 659622 [details] [diff] [review]:
-----------------------------------------------------------------

::: locales/en-US/chrome/chatzilla.properties
@@ +489,4 @@
>  
>  cmd.leave.format = Leave $channelName
>  cmd.leave.label  = &Leave
> +cmd.leave.params = [<channel-name>] [<reason>]

I don't believe the CommandManager will ever parse this as just having a reason, though I could be wrong, but it looks like the code in the command always validates the <channel-name> and merges it into the <reason> if it lacks a valid channel prefix (or, apparently, matches a channel name without prefix - something I'd forgotten about), so this is okay.

Comment 6

[James Ross]

http://hg.mozilla.org/chatzilla/rev/3056b93ac99d

Comment 7

[Tony Mechelynck [:tonymec]]

Mozilla/5.0 (X11; Linux x86_64; rv:18.0) Gecko/18.0 Firefox/18.0 SeaMonkey/2.15a1 ID:20120919171640 c-c:8fdbcb519211 m-c:0c8ac138706e

The Tinderbox waterfall designates this build as the first one started after comment #6.

After installing this hourly under /usr/local, starting it, browsing to file:///usr/local/seamonkey/distribution/extensions/ and clicking {59c81df5-4b7a-477b-912d-4e0fdf64e5f2}.xpi then restarting (to make sure the new ChatZilla build will be used)

Answer to "/help leave" has as its first line:
[USAGE]	leave [<channel-name>] [<reason>]

=> VERIFIED