Closed Bug 955156 Opened 10 years ago Closed 10 years ago

Separate strings for channel and user mode system messages

Categories

(Chat Core :: IRC, defect)

defect
Not set
minor

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: aleth, Assigned: aleth)

Details

Attachments

(1 file, 2 obsolete files)

*** Original post on bio 1727 at 2012-10-15 21:20:00 UTC ***

*** Due to BzAPI limitations, the initial description is in comment 1 ***
Attached patch Patch (obsolete) — Splinter Review
*** Original post on bio 1727 as attmnt 1964 at 2012-10-15 21:20:00 UTC ***

Recent changes mean we can improve the mode system messages slightly. (Replacing the mode letters with clearer text somehow is bug 954855 (bio 1420)).
Attachment #8353723 - Flags: review?(clokep)
*** Original post on bio 1727 at 2012-10-15 21:24:10 UTC ***

Comment on attachment 8353723 [details] [diff] [review] (bio-attmnt 1964)
Patch

>diff --git a/chrome/en-US/locale/en-US/chat/irc.properties b/chrome/en-US/locale/en-US/chat/irc.properties
>+#    %1$S is the nickname of the user whose mode was changed,
>+#    %2$S is the new mode and %3$S is who set the mode.
>+message.usermode=Mode %2$S for %1$S set by %3$S.
Shouldn't these be in number order? Should this message include the channel the mode was changed for?

>+#    %1$S is the new channel mode and %2$S is who set the mode.
>+message.channelmode=Channel mode %1$S set by %2$S.
*** Original post on bio 1727 at 2012-10-15 21:34:33 UTC ***

(In reply to comment #1)
> >+#    %1$S is the nickname of the user whose mode was changed,
> >+#    %2$S is the new mode and %3$S is who set the mode.
> >+message.usermode=Mode %2$S for %1$S set by %3$S.
> Shouldn't these be in number order? 
Probably, but I wanted to see what you thought of the strings first.

> >+#    %1$S is the new channel mode and %2$S is who set the mode.
> >+message.channelmode=Channel mode %1$S set by %2$S.
>Should this message include the channel the mode was changed for?
What for? The system message is always shown in the channel it refers to, unless I am missing something.
Comment on attachment 8353723 [details] [diff] [review]
Patch

*** Original change on bio 1727 attmnt 1964 at 2012-10-15 23:50:56 UTC ***

(In reply to comment #2)
> (In reply to comment #1)
> > >+#    %1$S is the nickname of the user whose mode was changed,
> > >+#    %2$S is the new mode and %3$S is who set the mode.
> > >+message.usermode=Mode %2$S for %1$S set by %3$S.
> > Shouldn't these be in number order? 
> Probably, but I wanted to see what you thought of the strings first.
> 
> > >+#    %1$S is the new channel mode and %2$S is who set the mode.
> > >+message.channelmode=Channel mode %1$S set by %2$S.
> >Should this message include the channel the mode was changed for?
> What for? The system message is always shown in the channel it refers to,
> unless I am missing something.
It is, I was just asking.

I think the strings seem fine then. Please make the change to the 1 & 2.
Attachment #8353723 - Flags: review?(clokep) → review-
Assignee: nobody → aleth
Status: NEW → ASSIGNED
Attached patch Patch (obsolete) — Splinter Review
*** Original post on bio 1727 as attmnt 1965 at 2012-10-16 10:54:00 UTC ***

Trying to make /mode better one small patch at a time... ;)
Attachment #8353724 - Flags: review?(clokep)
Comment on attachment 8353723 [details] [diff] [review]
Patch

*** Original change on bio 1727 attmnt 1964 at 2012-10-16 10:54:30 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353723 - Attachment is obsolete: true
Comment on attachment 8353724 [details] [diff] [review]
Patch

*** Original change on bio 1727 attmnt 1965 at 2012-10-16 10:55:53 UTC ***

Looks good to me. Thanks!
Attachment #8353724 - Flags: review?(clokep) → review+
Whiteboard: [checkin-needed]
Attached patch PatchSplinter Review
*** Original post on bio 1727 as attmnt 2002 at 2012-10-21 13:48:00 UTC ***

Patch had trailing whitespace.
Attachment #8353761 - Flags: review+
Comment on attachment 8353724 [details] [diff] [review]
Patch

*** Original change on bio 1727 attmnt 1965 at 2012-10-21 13:48:08 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353724 - Attachment is obsolete: true
*** Original post on bio 1727 at 2012-10-26 04:53:42 UTC ***

I'm not strongly against this patch, but I don't see how it really improves the situation.
What are the current expectations about bug 954855 (bio 1420)? If we are likely to fix it for 1.3, does it really make sense to change strings here that we intend to change again soon?
*** Original post on bio 1727 at 2012-10-26 14:05:44 UTC ***

(In reply to comment #7)
> What are the current expectations about bug 954855 (bio 1420)? If we are likely to fix it
> for 1.3, does it really make sense to change strings here that we intend to
> change again soon?
I don't think we have had a good idea yet on how to fix bug 954855 (bio 1420) (how/where to explain the various modes in the UI), so I doubt it will be ready for 1.3.

This patch doesn't do anything about explaining the modes themselves, but at least it makes the system messages about what is happening a bit less cryptic.
*** Original post on bio 1727 at 2012-11-03 04:20:32 UTC ***

Committed as http://hg.instantbird.org/instantbird/rev/f44c83a998ca

Thanks. :)
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Whiteboard: [checkin-needed]
Target Milestone: --- → 1.3
You need to log in before you can comment on or make changes to this bug.