Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

text collector doesn't collect when parameters are passed as a variable #11268

Open
2 tasks done
lekoala opened this issue Jun 4, 2024 · 5 comments
Open
2 tasks done

Comments

@lekoala
Copy link
Contributor

lekoala commented Jun 4, 2024

Module version(s) affected

5.x

Description

the method collectFromCode doesn't collect if the third argument is a variable instead of a plain array. This makes using _t not very practical if you reuse injection arguments in multiple strings.

eg from my project

Here is some code

$metaDescription = _t(
    'Controller.Someentity',
    "Meet {name} {address}",
    $args
);

is never collected

    $metaDescription = _t(
      'Controller.Someentity',
      "Meet {name} {address}",
      [
          'name' => $office->Name,
          'city' => $city,
          'address' => $address->StreetAddress . " " . $address->PostalCode . ". " . $address->City
      ];
);

works fine

How to reproduce

In a controller, have

        $var = ['some' => 'var'];
        $ImNOTCollected = _t('PageController.ImNOTCollected', 'test {some}', $var);
        $ImCollected = _t('PageController.ImCollected', 'test {some}', ['some' => 'var']);

Run text collector

=> ImNOTCollected is not there

Possible Solution

No response

Additional Context

No response

Validations

  • Check that there isn't already an issue that reports the same bug
  • Double check that your reproduction steps work in a fresh installation of silverstripe/installer (with any code examples you've provided)

PRs

@lekoala
Copy link
Contributor Author

lekoala commented Jun 4, 2024

current fix if you have the issue, if you can live with a silly syntax ;-)

$ImCollectedAgain = _t('PageController.ImNOTCollected', 'test {some}', [...$var]);

@GuySartorelli
Copy link
Member

I don't know that this is a bug so much as a desired enhancement. The text collector relies on what is currently a pretty crude hand-made static analysis. The capability to figure out what's in a variable would be a new capability, I think.

@lekoala
Copy link
Contributor Author

lekoala commented Jun 5, 2024

well, at least it could complain or make some kind of error. Or at least be documented here https://docs.silverstripe.org/en/5/developer_guides/i18n/#usage-in-php-files
at the moment it fails silently even though i have a _t function
even a regex would do a better job :)

@lekoala
Copy link
Contributor Author

lekoala commented Dec 23, 2024

it also doesn't work on this

_t(__CLASS__ . '.CANNOT_DELETE', "Not allowed to delete '$type' records"),

due to $type being a variable and not parsed correctly (but that is easily fixable by replacing . $type . by {type} and pass the type as context

@lekoala
Copy link
Contributor Author

lekoala commented Dec 23, 2024

@GuySartorelli i'm working on a PR that works with the relevant use cases. I think it makes a lot of sense in many places to be able to use variables as either the second or third argument

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants