Don't add the period outside a translated string
ClosedPublic
Actions

Authored by Imarok on Mar 9 2018, 12:11 AM.

Details

Reviewers

Commits

rP22234: Don't add the period outside a translated string

Summary

Don't add periods outside translated strings in network.js.
(Do not commit before the release cause of translators)

Test Plan

Should display the same as before.

Diff Detail

Repository

rP 0 A.D. Public Repository

Lint

Automatic diff as part of commit; lint not applicable.

Unit

Automatic diff as part of commit; unit tests not applicable.

Event Timeline

Imarok created this revision.Mar 9 2018, 12:11 AM

Harbormaster completed remote builds in B5415: Diff 6081.Mar 9 2018, 12:11 AM

elexis awarded a token.Mar 9 2018, 12:14 AM

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

Linter detected issues:
Executing section Default...
Executing section Source...
Executing section JS...
|    | [NORMAL] ESLintBear (space-before-function-paren):
|    | Unexpected space before function parentheses.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/messages.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/messages.js
| 470| 470| 		g_Selection.reset();
| 471| 471| 		g_Selection.addList(selection, false, cmd.type == "gather");
| 472| 472| 	},
| 473|    |-	"play-tracks": function (notification, player)
|    | 473|+	"play-tracks": function(notification, player)
| 474| 474| 	{
| 475| 475| 		if (notification.lock)
| 476| 476| 		{
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 3.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/messages.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/messages.js
| 577| 577| 	let notificationText =
| 578| 578| 		notification.instructions.reduce((instructions, item) =>
| 579| 579| 			instructions + (typeof item == "string" ? translate(item) : colorizeHotkey(translate(item.text), item.hotkey)),
| 580|    |-			"");
|    | 580|+		"");
| 581| 581| 
| 582| 582| 	Engine.GetGUIObjectByName("tutorialText").caption = g_TutorialMessages.concat(setStringTags(notificationText, g_TutorialNewMessageTags)).join("\n");
| 583| 583| 	g_TutorialMessages.push(notificationText);
|    | [NORMAL] ESLintBear (curly):
|    | Unnecessary { after 'if' condition.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/messages.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/messages.js
|1085|1085| 
|1086|1086| 	let message = "";
|1087|1087| 	if (notifyPhase == "all")
|1088|    |-	{
|    |1088|+	
|1089|1089| 		if (msg.phaseState == "started")
|1090|1090| 			message = translate("%(player)s is advancing to the %(phaseName)s.");
|1091|1091| 		else if (msg.phaseState == "aborted")
|1092|1092| 			message = translate("The %(phaseName)s of %(player)s has been aborted.");
|1093|    |-	}
|    |1093|+	
|1094|1094| 	if (msg.phaseState == "completed")
|1095|1095| 		message = translate("%(player)s has reached the %(phaseName)s.");
|1096|1096| 

binaries/data/mods/public/gui/session/messages.js
| 636| »   while·(true)
|    | [NORMAL] ESLintBear (no-constant-condition):
|    | Unexpected constant condition.

Link to build: https://jenkins.wildfiregames.com/job/differential/191/display/redirect

Harbormaster completed remote builds in B5415: Diff 6081.Mar 9 2018, 4:31 AM

correct and complete, tested with current svn, all ok
accepting for after the release

binaries/data/mods/public/gui/common/network.js
17 ↗	(On Diff #6081)	periods would probably a bit distracting in these string so ok to not have them

This revision is now accepted and ready to land.Mar 10 2018, 8:51 PM

Dunedan awarded a token.Mar 17 2018, 9:42 PM

elexis mentioned this in D1542: Replace periods with semicolons in network error reasons.May 28 2018, 11:22 PM

Nuke that Reason: Unknown Reason and don't show anything but the Connection lost sentence in that case.

Delete the Reason: string too rather than make it conditional? It's kind of self evident.

Connection to the server has been lost also doesn't really fit to everything that wasn't a network timeout.
I wonder if we should display it only in that case. But it's probably better to keep it, as some of them like "Playername in use" don't necessarily imply disconnection.

elexis added inline comments.Jun 6 2018, 4:37 PM

binaries/data/mods/public/gui/session/messages.js
158 ↗	(On Diff #6081)	Synchronizing

Apply comments of elexis

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

Linter detected issues:
Executing section Source...
Executing section JS...
|    | [NORMAL] ESLintBear (space-before-function-paren):
|    | Unexpected space before function parentheses.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/messages.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/messages.js
| 467| 467| 		g_Selection.reset();
| 468| 468| 		g_Selection.addList(selection, false, cmd.type == "gather");
| 469| 469| 	},
| 470|    |-	"play-tracks": function (notification, player)
|    | 470|+	"play-tracks": function(notification, player)
| 471| 471| 	{
| 472| 472| 		if (notification.lock)
| 473| 473| 		{
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 3.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/messages.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/messages.js
| 574| 574| 	let notificationText =
| 575| 575| 		notification.instructions.reduce((instructions, item) =>
| 576| 576| 			instructions + (typeof item == "string" ? translate(item) : colorizeHotkey(translate(item.text), item.hotkey)),
| 577|    |-			"");
|    | 577|+		"");
| 578| 578| 
| 579| 579| 	Engine.GetGUIObjectByName("tutorialText").caption = g_TutorialMessages.concat(setStringTags(notificationText, g_TutorialNewMessageTags)).join("\n");
| 580| 580| 	g_TutorialMessages.push(notificationText);
|    | [NORMAL] ESLintBear (curly):
|    | Unnecessary { after 'if' condition.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/messages.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/messages.js
|1082|1082| 
|1083|1083| 	let message = "";
|1084|1084| 	if (notifyPhase == "all")
|1085|    |-	{
|    |1085|+	
|1086|1086| 		if (msg.phaseState == "started")
|1087|1087| 			message = translate("%(player)s is advancing to the %(phaseName)s.");
|1088|1088| 		else if (msg.phaseState == "aborted")
|1089|1089| 			message = translate("The %(phaseName)s of %(player)s has been aborted.");
|1090|    |-	}
|    |1090|+	
|1091|1091| 	if (msg.phaseState == "completed")
|1092|1092| 		message = translate("%(player)s has reached the %(phaseName)s.");
|1093|1093| 
Executing section cli...

Link to build: https://jenkins.wildfiregames.com/job/differential/1281/display/redirect

Harbormaster completed remote builds in B7294: Diff 7850.Apr 25 2019, 6:36 PM

Closed by commit rP22234: Don't add the period outside a translated string (authored by Imarok). · Explain WhyApr 28 2019, 5:26 PM

This revision was automatically updated to reflect the committed changes.