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

scripts/deploy.sh: add handmade linter for translation-related json files to check for tabs and odd spaces #2064

Merged
merged 10 commits into from
Feb 1, 2025

Conversation

ia
Copy link
Collaborator

@ia ia commented Feb 1, 2025

  • Please check if the PR fulfills these requirements
  • The changes have been tested locally
  • There are no breaking changes
  • What kind of change does this PR introduce?
    Implement very basic check for tabs and odd spaces in json files.

  • What is the current behavior?
    There are no any automatic code style checks for json files.

  • What is the new behavior (if this is a feature change)?
    With every modification json files will be checked for tabs and odd spaces as part of CI/autotesting process.

  • Other information:
    There are some full-featured json linters, but the ones I saw required nodejs/npm and other "heavy" toolsets, so for basic checks two calls of grep inside bash should be enough: it checks for tab as octet number 11 of tab control character (9 in dec/hex from ASCII table), and for odd amount of spaces as indent. Both grep calls tested successfully on MacOS 10.10.5 with GNU bash and BSD grep. It even already helped to discover existed mistype - error message example here.

@ia ia added Translations Linguas, i18n & l10. Build System make/bash/python, github CI/CD & pals. labels Feb 1, 2025
@ia ia requested review from Ralim and discip February 1, 2025 00:38
@ia ia self-assigned this Feb 1, 2025
@@ -97,7 +97,7 @@ docs_links()
md="README.md"
ver_md="$(grep -c "${ver_git}" "${md}")"
ret=0
if [ "${ver_md}" -ne 0 ]; then
if [ "${ver_md}" -eq 0 ]; then
Copy link
Collaborator Author

@ia ia Feb 1, 2025

Choose a reason for hiding this comment

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

This is the right way, and it was wrong, but it didn't trigger any errors, because at this point local directory was Translations/ (because I forgot to add cd .., which is now on the line 119 120 below).

UPD: also added a couple of checks hopefully to avoid this in the future.

@ia ia enabled auto-merge (squash) February 1, 2025 00:55
return 1
fi;

grep -nH -e "^ [^ ]" -e "^ [^ ]" -e "^ [^ ]" -e "^ [^ ]" -e "^ [^ ]" -e "^ [^ ]" Translations/translation*.json
Copy link

Choose a reason for hiding this comment

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

maybe less efficient computational wise, but at least we don't need to second guess we counted the white spaces in the grep patterns

awk -F '[^ ].*' 'BEGIN{ex=1} length($1) % 2 == 1 {ex=0; printf "%s:%s: %s\n", FILENAME, FNR, $0} END{exit ex}' Translations/translation*.json

Copy link

@yo3fxy yo3fxy Feb 1, 2025

Choose a reason for hiding this comment

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

or variant of your grep command

grep -nEH -e "^ [^ ]" -e "^ {3}[^ ]" -e "^ {5}[^ ]" -e "^ {7}[^ ]" -e "^ [9]{^ ]" -e "^ {11}[^ ]" Translations/translation*.json

edit: more compact version of previous grep alternative

grep -nEH -e "^( {1}| {3}| {5}| {7}| {9}| {11})[^ ]" Translations/translation*.json

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

maybe less efficient computational wise, but at least we don't need to second guess we counted the white spaces in the grep patterns

awk [...]

I use awk myself from time to time for very basic patterns, mostly to filter content based on custom separators. But I try not to stuff too much extra tools here, because most of builds & tests taking place inside Alpine Linux environment, which is very minimal and every extra tool require to be installed as a separate package. So, since we already use grep, I try to utilize it. Hence, that's why I stick to grep here.

Copy link
Collaborator Author

@ia ia Feb 1, 2025

Choose a reason for hiding this comment

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

edit: more compact version of previous grep alternative

grep -nEH -e "^( {1}| {3}| {5}| {7}| {9}| {11})[^ ]" Translations/translation*.json

Thanks for help! I will add it as soon as possible. I was thinking to implement this in similar way, but for some reason I thought that just leaving spaces will be more easy to understand in the future, but I really like your suggestion.

UPD: Done. I checked briefly on macOS with BSD grep - works as well.

@ia ia merged commit 22d0676 into Ralim:dev Feb 1, 2025
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Build System make/bash/python, github CI/CD & pals. Translations Linguas, i18n & l10.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants