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

Add linting/warnings #117

Closed
samhh opened this issue May 4, 2022 · 5 comments
Closed

Add linting/warnings #117

samhh opened this issue May 4, 2022 · 5 comments

Comments

@samhh
Copy link
Contributor

samhh commented May 4, 2022

This could lint against:

* I feel like this should be configurable, but that's probably overkill for now.

Notably these should be warnings, not errors. As such they need to be expressed via stdout, meaning we may want to start writing to files directly. Relevant: #1, #25

@OliverJAsh
Copy link
Member

  • Advanced usages that may confuse translators e.g. more than one select/plural/etc in a message *

Note that in most cases these advanced usages only start to appear after we flatten—so we would want to lint the flattened output?

@samhh
Copy link
Contributor Author

samhh commented May 5, 2022

Note that in most cases these advanced usages only start to appear after we flatten—so we would want to lint the flattened output?

If we lint against {x, select, foo {}} {y, select, bar {}} in the knowledge that with flattening it becomes nested then that accomplishes the same thing.

@samhh
Copy link
Contributor Author

samhh commented May 10, 2022

TIL that > redirects stdout but not stderr, so printing to stdout doesn't block this ticket. We just need to print lint warnings to stderr. (What about the exit code?)

#!/bin/sh

echo stdout!
echo stderr... >&2
$ ./script.sh > out.txt
stderr...
$ cat out.txt
stdout!

Alternatively we could add a distinct lint subcommand. That'll be a bit slower as it means parsing everything again, however that does strike me as potentially the cleanest approach.

@samhh
Copy link
Contributor Author

samhh commented May 11, 2022

This can either be a subcommand in the preexisting binary or a new binary - it doesn't make much difference in code as everything that matters is in the shared library.

I'm tempted to say that it should be a new binary given how opinionated towards our own needs it'll be. Current rule plans and whether they're not too specific to our needs:

@samhh
Copy link
Contributor Author

samhh commented Jun 23, 2022

We now have an internal linter covering the nested/complexity rule as of #123. #128 will cover Unicode. Number skeletons can wait until the outcome of #112. Issues raised for a non-internal linter and rules with #135 and #136.

@samhh samhh closed this as completed Jun 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants