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

revise validate-extension-inputs.js to detect problems with percent i… #138

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

benjiwheeler
Copy link
Contributor

Resolves #137

Changes

  • Adds functionality to the validate-extension-inputs.js script so it can check for the presence of the correct inputs of the form "/%d+/".
  • makes validate-extension-inputs.js take a commandline arg to set which directory to use as input
  • adds validate-extension-inputs.js to the tests run on editor/blocks translations

Copy link
Contributor

@chrisgarrity chrisgarrity left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is really helpful. One simple change is to replace all the console.log with process.stdout.write. I think it's a bit clearer for command line scripts. I wouldn't use process.stderr because it isn't an error in the script itself.

Also, would it make sense to rename the script 'validate-inputs.js' as it's not just for extensions any more.

);
} catch (err) {
numLocaleErrors++;
console.error(err.message + '\n'); // eslint-disable-line no-console
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
console.error(err.message + '\n'); // eslint-disable-line no-console
process.stdout.write(err.message + '\n');

It's better to use process.stdout for this type of script (also doesn't trigger eslint)

@benjiwheeler
Copy link
Contributor Author

@chrisgarrity, we're assigning this to you to decide what to do with this one! Hope it's helpful!

@chrisgarrity chrisgarrity removed their assignment Sep 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

translation validation tests allow errors with "%1"-type inputs to go through
2 participants