wp-cli / i18n-command

Provides internationalization tools for WordPress projects.
MIT License
96 stars 52 forks source link

Improve theme patterns and styles lookup #320

Closed swissspidy closed 2 years ago

swissspidy commented 2 years ago

This is in response to https://github.com/wp-cli/i18n-command/pull/312/files#r863120121

Related:

The downside of this change is that now pattern headers are also extracted from files outside the patterns directory. Can we live with that?

swissspidy commented 2 years ago

cc @ocean90

swissspidy commented 2 years ago

Perhaps in the extractor we can add a separate check to just extract the strings if the current file is in a patterns directory.

ocean90 commented 2 years ago

The downside of this change is that now pattern headers are also extracted from files outside the patterns directory. Can we live with that?

Ah, now it makes sense, why it was done like that. To me this sounds like this task should then only look in patterns and only extract the headers and no other strings. But it seems the current extractor doesn't support this?

I just noticed that we have the same issue with styles from https://github.com/wp-cli/i18n-command/commit/752a4d4dd14a2d6c93dba5ae4410677c0b5d3c34#diff-1197c2ddc1381faef42d8ac1eaf93f93a7b205617cca46cf7611a7462c64f9eeR690, see https://wordpress.slack.com/archives/C02RP50LK/p1651528912931309.

swissspidy commented 2 years ago

Yeah that would also work but I think you‘re right, it doesn‘t support that. Plus it would mean potentially unnecessarily scanning a file twice. Checking for the expected folder structure within the extractor could work for both patterns and styles.

I‘ll neee to experiment with that.

ocean90 commented 2 years ago

Plus it would mean potentially unnecessarily scanning a file twice.

That would only be a file_get_contents() + get_file_data_from_string() call right? Sounds okay for me.

What about adding a skip_default option which can be set to true to prevent running https://github.com/wp-cli/i18n-command/blob/bcb1a8159679cafdf1da884dbe5830122bae2c4d/src/IterableCodeExtractor.php#L93

swissspidy commented 2 years ago

Tried a different approach now. The downside is that it hardcodes the logic for included folders and file names, but at least it does not mess with the include/exclude options nor scan files twice.