Page MenuHomePhabricator

Fix `pht` method calls
ClosedPublic

Authored by joshuaspence on Feb 4 2015, 10:42 PM.
Tags
None
Referenced Files
F12829066: D11683.id28218.diff
Thu, Mar 28, 11:14 AM
Unknown Object (File)
Sat, Mar 16, 5:12 PM
Unknown Object (File)
Fri, Mar 15, 3:18 PM
Unknown Object (File)
Feb 11 2024, 10:29 AM
Unknown Object (File)
Feb 1 2024, 8:03 PM
Unknown Object (File)
Jan 20 2024, 4:46 PM
Unknown Object (File)
Jan 16 2024, 4:19 PM
Unknown Object (File)
Jan 15 2024, 3:09 PM
Subscribers

Details

Summary

Ref T7046. This is mainly a proof-of-concept for D11661.

Test Plan

arc lint

Diff Detail

Repository
rARC Arcanist
Branch
master
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 4404
Build 4417: [Placeholder Plan] Wait for 30 Seconds

Event Timeline

joshuaspence retitled this revision from to Fix `pht` method calls.
joshuaspence updated this object.
joshuaspence edited the test plan for this revision. (Show Details)
joshuaspence added a reviewer: epriestley.
epriestley edited edge metadata.

One inline, let me know if that's clear? These might also have translations specified already that could need to be cleaned up.

src/workflow/ArcanistCommitWorkflow.php
215–219

These aren't wrong, they just aren't in the correct form.

The intent is to allow translations to choose pluralization, but the way this was done (likely by me) is very confusing because it doesn't use the parameter in the string. This technically works (translations can make alternate-text decisions based on extra parameters), it's just super derpy and we/I stopped doing this a long time ago.

Better would be to include the count in the string:

%s locally modified path(s) are not included in this revision:

...then make the translations, not the original text, omit the number in English:

A locally modified path is not included in this revision:
Locally modified paths are not included in this revision:

This style is better in every way, and especially better now that you've built lint for it, so this doesn't create any kind of special case that we need to think about, lint-wise.

This revision now requires changes to proceed.Feb 9 2015, 2:23 PM

Sounds reasonable. Where are the base translations for these strings anyway? PhabricatorBaseEnglishTranslation lives in rP.

Sounds reasonable. Where are the base translations for these strings anyway? PhabricatorBaseEnglishTranslation lives in rP.

src/workflow/ArcanistCommitWorkflow.php
216

If we are dropping the number from the translated text, we could just use %d instead of %s.

Oh, looks like they're just loose at top level in __init_script__.php. Yeuch.

epriestley edited edge metadata.

The rest of this stuff looks solid.

This revision is now accepted and ready to land.Feb 9 2015, 8:08 PM
joshuaspence edited edge metadata.

Update base translations

This revision was automatically updated to reflect the committed changes.