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

Notify about warnings from make-pot command #48

Open
swissspidy opened this issue Jul 4, 2018 · 8 comments
Open

Notify about warnings from make-pot command #48

swissspidy opened this issue Jul 4, 2018 · 8 comments
Labels
[Type] Question Support requests and other types of questions
Milestone

Comments

@swissspidy
Copy link
Collaborator

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 swissspidy added the [Type] Question Support requests and other types of questions label Jul 4, 2018
@grappler
Copy link
Contributor

grappler commented Jul 4, 2018

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' );

@ocean90
Copy link
Member

ocean90 commented Jul 4, 2018

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

@ocean90
Copy link
Member

ocean90 commented Jul 4, 2018

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?

@swissspidy
Copy link
Collaborator Author

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
Copy link
Collaborator Author

Got some feedback from Alain about this: wp-cli/i18n-command#33 (comment)

@swissspidy
Copy link
Collaborator Author

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

@swissspidy
Copy link
Collaborator Author

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.

@ocean90 ocean90 modified the milestones: 3.1.0, 3.2.0 Jul 20, 2020
@swissspidy
Copy link
Collaborator Author

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 wp-cli/i18n-command#154):

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

Maybe it's helpful.

@ocean90 ocean90 modified the milestones: 3.2.0, 3.3.0 Jan 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Question Support requests and other types of questions
Projects
None yet
Development

No branches or pull requests

3 participants