Skip to content

[3.0] Give these strings the named arguments they ask for - #9408

Open
albertlast wants to merge 1 commit into
SimpleMachines:release-3.0from
albertlast:3.0/icu-named-arguments
Open

[3.0] Give these strings the named arguments they ask for#9408
albertlast wants to merge 1 commit into
SimpleMachines:release-3.0from
albertlast:3.0/icu-named-arguments

Conversation

@albertlast

Copy link
Copy Markdown
Collaborator

Description

A Lang::getTxt() call passes either a positional list or a named map, and the string decides which is right: {0} wants a list, {name} wants a map. Five calls pass a list to a string with named placeholders, so nothing is substituted and the placeholder is printed as written.

Moving a board is the worst of them

Every destination in the list reads the same, so there is no way to tell them apart. Admin → Boards → Move, before and after:

Before {name}         →   Before General Discussion
After {name}          →   After General Discussion
Sub-board of {name}   →   Sub-board of General Discussion

Four calls in Actions/Admin/Boards.php are affected. The same file already gets it right about a hundred lines further down, for the board order drop-down:

'name' => Lang::getTxt('mboards_order_after', ['name' => $tree->name], file: 'ManageBoards'),
The other two
  • Actions/Admin/Logs.php — the error log's own description tells you to "click the {remove} button at the bottom of the page". Languages/en_US/Admin.php even carries a translator note above the string: "Do not translate '{remove}'. It will be automatically replaced with the value of $txt['remove']." It was not being replaced. Now reads "click the Remove button".
  • Sources/User.php — the ban notice says "This ban is set to expire {datetime}." By inspection; I did not stage a ban with an expiry to watch it.
How these were found

A sweep matching each getTxt('key', [ … ]) call against the placeholders in the string it names, and comparing the shape of the literal array — top-level => or not — against whether the string wants named or positional arguments. Eight hits, three of them false: ordinal_spellout and guest_plural (twice) are ICU selectordinal / plural patterns whose branch bodies (=1 {first}, one {guest}) look like placeholders to a regex but are not.

This is the same family as #9400, where an ICU plural argument was given HTML and read as zero. If the unimported-class sweep offered in #9395 is of interest as a CI step, this one and the language-file sweep in #9397 belong with it — none of the three is visible to phplint or php-cs-fixer, and all three fail silently.

Issues References (Fixes|Related|Closes)

Related to #7933

A getTxt() call passes either a positional list or a named map, and the
string decides which is right: {0} wants a list, {name} wants a map. Five
calls pass a list to a string with named placeholders, so nothing is
substituted and the placeholder is printed as is.

Move a board and every destination in the list reads the same:

  Before {name}      instead of   Before General Discussion
  After {name}                    After General Discussion
  Sub-board of {name}             Sub-board of General Discussion

The same file already gets this right a hundred lines further down, in
mboards_order_after for the board order drop down, which passes
['name' => $tree->name].

The other two are the error log's description, which tells you to "click the
{remove} button", and the ban notice, which says a ban expires {datetime}.

Signed-off-by: Mathias Papenbrock <mathiaspapealbert@hotmail.com>
Signed-off-by: albertlast <mathiaspapealbert@hotmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant