Page MenuHomeWildfire Games

Fix lobby IRC translation comment
Needs ReviewPublic

Authored by temple on Dec 9 2017, 8:35 PM.
This revision needs review, but all reviewers have resigned.

Details

Reviewers
elexis
Summary
#: gui/lobby/lobby.js:1382
#, javascript-format
msgctxt "lobby private message"
msgid "(%(private)s) <%(sender)s>"
msgstr ""
Test Plan

Searching for failed translation comments came up with just this one.

Diff Detail

Repository
rP 0 A.D. Public Repository
Branch
/ps/trunk
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 4102
Build 7211: Vulcan BuildJenkins
Build 7210: arc lint + arc unit

Event Timeline

temple created this revision.Dec 9 2017, 8:35 PM
Owners added a subscriber: Restricted Owners Package.Dec 9 2017, 8:35 PM
elexis added a comment.Dec 9 2017, 8:37 PM

Sure that was the string it was written for? Not the one below?

Vulcan added a subscriber: Vulcan.Dec 9 2017, 8:37 PM

Successful build - Chance fights ever on the side of the prudent.

Updating workspaces...
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (308 tests)....................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (308 tests)....................................................................................................................................................................................................................................................................................................................OK!
Checking XML files...
temple added a comment.Dec 9 2017, 9:08 PM

Oh, I didn't look carefully. The one a few lines later has the same translation message in other places, so maybe you're right.

#. Translation: IRC message prefix.
#: gui/lobby/lobby.js:1337 gui/lobby/lobby.js:1358 gui/lobby/lobby.js:1386
#, javascript-format
msgid "<%(sender)s>"
msgstr ""

The one here is also an IRC message prefix, but for private messages, which is what the context says, but it would probably be better to put that into the comment. My impression was that context was used when you wanted to distinguish this string from the same string used elsewhere, and that's not happening in this case.

Notice that the next line is also translated, and maybe it deserves a translation comment.

#: gui/lobby/lobby.js:1382
msgid "Private"
msgstr ""
temple updated this revision to Diff 4678.Dec 9 2017, 9:39 PM

Was more careful.
History at rP18375.

In D1136#45386, @temple wrote:

Oh, I didn't look carefully. The one a few lines later has the same translation message in other places, so maybe you're right.

I don't think we need a crystall ball to find some commit where someone familiar might have broken that comment :-)

My impression was that context was used when you wanted to distinguish this string from the same string used elsewhere, and that's not happening in this case.

Correct, that's the use case of the translation context afaik.

Notice that the next line is also translated, and maybe it deserves a translation comment.

What the translation really deserves is someone answering the questions on transifex.com. If noone complained, it's not so bad.

Successful build - Chance fights ever on the side of the prudent.

Updating workspaces...
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (308 tests)....................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (308 tests)....................................................................................................................................................................................................................................................................................................................OK!
Checking XML files...

rP18375 is the culprit.

binaries/data/mods/public/gui/lobby/lobby.js
1301

Last time I came around this I didn't find a nice way to obliterate it. Would be nice to find some JS split magic for it and have it inlined.

1309

formatChatMessage?

1385

I've always wondered why we speak of IRC messages in XMPP. Can we shoot at it? Just replacing IRC with chat everywhere should do.

elexis retitled this revision from Fix another translation comment to Fix lobby IRC translation comment.Dec 10 2017, 4:44 AM
elexis resigned from this revision.Dec 12 2017, 8:36 PM
temple updated this revision to Diff 4750.Dec 12 2017, 8:57 PM

irc -> chat

Successful build - Chance fights ever on the side of the prudent.

Updating workspaces...
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (308 tests)....................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (308 tests)....................................................................................................................................................................................................................................................................................................................OK!
Checking XML files...
temple added inline comments.Dec 12 2017, 9:02 PM
binaries/data/mods/public/gui/lobby/lobby.js
1301

messages.js uses

	// Split addressee command and message-text
	msg.cmd = msg.text.split(/\s/)[0];
	msg.text = msg.text.substr(msg.cmd.length + 1);

You could do split to get an array of words then add back spaces later.
I don't have opinions on these things.

1304

Don't know what to do here, if anything.

bb added a subscriber: bb.Dec 23 2017, 11:17 PM

All cases of IRC message removed from the code

some questions, probably no changes required though:

binaries/data/mods/public/gui/lobby/lobby.js
1295

(wondering why single ' are used and not ", but out of scope I guess)

1304

the comment seems valid, so nothing is ok imo

1327

why is here some quotation and in previous comment not?

1336

and why isn't here the /me type explanation?