[mlutfy]

Adds basic support for translating any af-text element in Afform. Tests included, but only one of the test cases have been implemented. There is overlap with work being done by @samuelsov and @totten in core.

Todo:

• Have a list of xpath/selectors that we should translate (the list should be in core, so that civistrings can run on various versions of core).
• Implement the remaining test cases

Although I wonder if it will be possible to share a list of selectors with core, considering that most cases are obscure things such as:

<af-field name="Something.Something" defn="{label: 'Very Something Something'}" />

which I suspect we will not be able to parse with phpQuery?

[totten]

@mlutfy I've rebased and fixed some conflicts on composer.json.

Additionally, I believe that the parser on the afform side evolved after this draft was introduced. It seems like the easiest thing is... to copy in the afform utility (StringVisitor.php) so that we get the exact same extractions.

Pushed all that up.

But there are still some oddities. If you do composer install and ./bin/phpunit, that'll point toward them.

[shaneonabike]

@totten So we built out a bunch of code for FormBuilder to basically parse this for translations. Are we able to call that parsing code instead of rebuilding it here or this needs to be independent? If so, I might upgrade @mlutfy code to basically use what we setup or copy/paste the same processing in his code.

[mlutfy]

This would probably be overkill in the short-term: I suspect it would be best if the extraction code was in a PHP library, and then civistrings / civicrm would use that.

Since it's not a ton of code, maybe it's not worth it, and we can just copy-paste for now?

Although I see the original post had mentions of xpaths, so perhaps that part is version-dependent. @shaneonabike Can you point me to the core code, or update this PR (if you have permissions)?

[shaneonabike]

Perso I agree that calling it is better for maintenance reasons. We are surely going to add more and each time we would need to backport which would be a pain.

The PR is https://github.com/civicrm/civicrm-core/pull/32859/files

The String parser (helper function) is found in ext/afform/core/Civi/Afform/StringVisitor.php, which is basically processed via ext/afform/core/Civi/Api4/Utils/AfformSaveTrait.php (which basically processes and then dumps it to the database via saveTranslations

[shaneonabike]

@mlutfy happy to help OR modify this but I would need to know what the best practices are for basically calling Civi functions (if there are any)

[totten]

(@shaneonabike) Are we able to call that parsing code instead of rebuilding it here or this needs to be independent?

There are always multiple ways to solve a problem. :)

Using the live copy from civicrm-core would require a different posture for civistrings. Right now, you can run civistrings as standalone tool (e.g. in a CI workflow) without needing a full deployment of CiviCRM.

• One could update civistrings.git to boot Civi (much like cv or civix). There's a library civicrm/cv-lib with helpers to do this. (That's what civix does.)
• One could rework civistrings as a cv plugin (e.g. cv civistrings [files....]). (But this is probably a messier -- because civistrings has some of its own special dependencies.)

(@mlutfy) This would probably be overkill in the short-term: I suspect it would be best if the extraction code was in a PHP library, and then civistrings / civicrm would use that.

Another possible middle-ground is for civistrings to download the StringVisitor.php file from github.com/civicrm/civicrm-core/. It's a bit like using a library (we pull in the code here; and we need to periodically republish civistrings.phar), but it's only one file, and you don't have to maintain a separate git project.

civix does a similar thing when it uses Mixlib.php from civicrm-core.

This requires some discipline in the maintenance of civicrm-core.git:StringVisitor.php -- the file needs to remain self-contained/portable.

[shaneonabike]

@totten I like your download method. Presently, it is pretty self-contained and perhaps after setting it up here we add a comment in StringVisitor.php that it is being retrieved by civistrings? Presently, I think we did set it up to be a one-time stop shop for handling parsing so that has it's advantages. Especially, later on when we finish Searchkit.

Is that an avenue you think is reasonable to explore then? I can see what I can do to make that happen perhaps

[totten]

@shaneonabike Yep, I think that's a good avenue.

[mlutfy]

@totten I added StringVisitor.php in composer.json. It seems to work (I deleted the original PHP file).

Does this do anything? (I expected it to fail)

        "compile": [
            {
                "title": "Download StringVisitor (<comment>mixin-backports.php</comment>)",
                "run": "@php-method \\CRM\\CivixBundle\\ComposerCompile::downloadMixins"
            }
        ],

[totten]

@mlutfy Ah, yeah, you can just delete the compile clause.

For explanation, it's a bit like this:

• composer-downloads-plugin reads the extra.download section. (But if the plugin isn't here, then it doesn't do anything...)
  • Specifically, in this branch, this appears to be the live code.
• composer-compile-plugin reads the extra.compile section. (But if the plugin isn't here, then it doesn't do anything...)
  • Generally, the compile-plugin will let you call PHP methods/classes/scripts.
  • Specifically, in civix, it calls a function that does some extra download logic for some specific files.
  • But it's a more specialized thing.
• If composer-downloads-plugin works for civistrings, then use that. 👍

[totten]

This line gives a name for the download task. Since you're downloading StringVisitor.php (not Mixlib.php), I'd suggest a name like:

Suggested change

	"mixlib": {
	"StringVisitor": {

[mlutfy]

@totten Thanks! I implemented the changes you suggested.

[totten]

I'm wondering if there's any reason not merge?

I guess we should run phpunit again. With this branch, I'm seeing a regression in processing examples/ex5.html (which is expected to produce examples/ex5.pot). It currently produces:

1) Civi\Strings\Command\ExtractCommandTest::testExecute with data set #4 (array('examples/ex5.html'), 'examples/ex5.pot')
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
 '#: examples/ex5.html\n
+msgid "ts('Attribute with double+single-quotes')"\n
+msgstr ""\n
+\n
+#: examples/ex5.html\n
+msgid "ts(\"Attribute with single+double-quotes\")"\n
+msgstr ""\n
+\n
+#: examples/ex5.html\n
+msgid "ts('Attribute with double+single-quotes and %1', {1: 'foo'})"\n
+msgstr ""\n
+\n
+#: examples/ex5.html\n
+msgid "ts(\"Attribute with single+double-quotes and %1\", {1: \"foo\"})"\n
+msgstr ""\n
+\n
+#: examples/ex5.html\n
 msgid "Text node with double-quotes"\n
 msgstr ""\n
 \n

/home/totten/src/civistrings/tests/Command/ExtractCommandTest.php:48

(Aside: On the master branch, the tests have other issues -- related to #22. We should do something along these lines to make it more obvious. In any case, if you focus on this branch and do not rebase, then the test output should be useful.)

[mlutfy]

Took me a while to see it, but it extracts strings such as:

msgid "ts('Attribute with double+single-quotes')"
msgstr ""

i.e. with the ts() markup, which is incorrect.

[totten]

I merged some changes into master that caused on conflicts on composer.json. Rebased and force-pushed to resolve them.

[samuelsov]

Took me a while to see it, but it extracts strings such as:

msgid "ts('Attribute with double+single-quotes')"
msgstr ""

i.e. with the ts() markup, which is incorrect.

Probably best to fix the StringVisitor::isWorthy() function.

This is an attempt at this -> civicrm/civicrm-core#34303

[shaneonabike]

jenkins retest this please

[mlutfy]

I updated StringVisitor, but it's now failing with:

1) Civi\Strings\Command\ExtractCommandTest::testExecute with data set #4 (array('examples/ex5.html'), 'examples/ex5.pot')
Error: Call to a member function find() on array

/app/extern/Civi/Afform/StringVisitor.php:85
/app/src/Parser/HtmlParser.php:25
/app/src/Command/ExtractCommand.php:181
/app/src/Command/ExtractCommand.php:104
/app/vendor/symfony/console/Command/Command.php:298
/app/vendor/symfony/console/Tester/CommandTester.php:74
/app/tests/Command/ExtractCommandTest.php:45

[shaneonabike]

@totten could you help out with the issue above? ^^

[eileenmcnaughton]

@mlutfy this needs a rebase

[eileenmcnaughton]

@mlutfy what is the status here - I see another PR says it's waiting on this but this is looking stalled

[mattwire]

@mlutfy @eileenmcnaughton @shaneonabike @samuelsov I rebased and fixed the stringVisitor issue (with a little help from claude)

[eileenmcnaughton]

@mattwire just reading this comment "which I suspect we will not be able to parse with phpQuery?"

phpQuery isn't maintained - I did look at what replacing it with a few functions in core might look like

civicrm/civicrm-core#37932

[eileenmcnaughton]

@colemanw you have a PR that depends on this - is this (& by extension your PR) mergeable now?