traduttore icon indicating copy to clipboard operation
traduttore copied to clipboard

Notify about warnings from make-pot command

Open swissspidy opened this issue 6 years ago • 8 comments

When running wp i18n make-pot, the script notifies you when an extracted string has two or more different translator comments.

See https://github.com/wp-cli/i18n-command/blob/7e11d8c2ead4787997c41fb77809f7c90ef787ca/src/MakePotCommand.php#L315-L321 and https://github.com/wp-cli/i18n-command/blob/7e11d8c2ead4787997c41fb77809f7c90ef787ca/features/makepot.feature#L528-L556.

This is usually a sign of a developer mistake, as the string would only be imported once in GlotPress, but have 2 comments added to it.

Here's an (over simplified) example:

/* translators: Translators 1! */
sprintf( __( 'Hello World', 'foo-plugin' );

/* Translators: Translators 2! */
__( 'Hello World', 'foo-plugin' );

What if Traduttore would send a Slack notification when such a warning is encountered? (*)

The same goes for other warnings:

  • Files that can't be opened
  • Files that can't be parsed and need to be reviewed and perhaps excluded
  • The command fails for another reason and exits with a non-zero status

(*) Now that I think about it, this could also work as a HM Linter integration 🤯

swissspidy avatar Jul 04 '18 08:07 swissspidy

I was going to say that this would be best for PHPCS but then I realised that would not be possible the the two strings were in the same file. The problem is that PHPCS has no memory across files.

General error reporting when running wp i18n make-pot would be good.

This being en error is not always the case. The following would be a valid case.

/* Translators: Noun */
__( 'Post', 'my-theme' );
/* Translators: Verb */
__( 'Post' 'my-theme' );

grappler avatar Jul 04 '18 08:07 grappler

@grappler It's not valid, it should be _x( 'Post', 'noun', 'my-theme' ) and _x( 'Post', 'verb', 'my-theme' ).

ocean90 avatar Jul 04 '18 08:07 ocean90

Including warnings in the Slack notification for traduttore_updated_from_github sounds like a good idea. I guess wp i18n make-pot needs another output format so it's machine-readable?

ocean90 avatar Jul 04 '18 08:07 ocean90

I guess wp i18n make-pot needs another output format so it's machine-readable?

Probably, unless we want to parse the console output using regex or something. I'll look into it to see whether we can extend the command for that.

swissspidy avatar Jul 04 '18 09:07 swissspidy

Got some feedback from Alain about this: https://github.com/wp-cli/i18n-command/issues/33#issuecomment-402434724

swissspidy avatar Jul 04 '18 10:07 swissspidy

Note: the Slack WordPress plugin isn't being maintained that actively and still doesn't support attachments: https://github.com/gedex/wp-slack/issues/8.

swissspidy avatar Jul 04 '18 11:07 swissspidy

I think in the meantime we could try extracting the warnings using regex. They're pretty straightforward.

Looking at https://api.slack.com/docs/message-attachments it just seems like there's no good way to show these warnings in a Slack message, e.g. in a collapsible field or something. Also it's not very user friendly to just have a dozens of warnings in that

Maybe we could just show the total number of warnings in the Slack message and link to a page where all translations with warnings are shown?

For this we'd need to store these warnings in the database and would gain the ability to display them in the UI using GP_Translation_Warnings?

Not sure if that's possible though using the filters in GlotPress, cc @ocean90.

swissspidy avatar Nov 13 '18 08:11 swissspidy

Here's some hacky bash code I put together the other day to retrieve all warnings by WP-CLI (and filter out false positives caused by https://github.com/wp-cli/i18n-command/issues/154):

https://github.com/google/web-stories-wp/blob/fc38c6e046e90c8b48fd5ef410a8a45d6ff02078/.github/workflows/lint-i18n.yml#L113-L142

Maybe it's helpful.

swissspidy avatar Jun 04 '21 08:06 swissspidy