Copilot commented on code in PR #4080:
URL: https://github.com/apache/streampipes/pull/4080#discussion_r2655221903
##########
ui/scripts/sort-translations.js:
##########
@@ -0,0 +1,18 @@
+const fs = require('fs');
+
+const file = process.argv[2];
+if (!file) {
+ console.error('Usage: node sort-translations.js <file>');
+ process.exit(1);
+}
+
+const json = JSON.parse(fs.readFileSync(file, 'utf8'));
+
Review Comment:
The script doesn't validate that the file contains valid JSON before
attempting to parse it. If the file is corrupted or contains invalid JSON,
JSON.parse will throw a SyntaxError that isn't caught. Consider wrapping the
JSON.parse call in a try-catch block and providing a clear error message.
```suggestion
let json;
try {
json = JSON.parse(fs.readFileSync(file, 'utf8'));
} catch (err) {
console.error(`Error: File "${file}" does not contain valid JSON:
${err.message}`);
process.exit(1);
}
```
##########
ui/package.json:
##########
@@ -25,7 +25,7 @@
"test-cypress-smoke": "npx cypress run --spec
'cypress/tests/**/*.smoke.spec.ts' --config baseUrl=http://localhost:8082";,
"test-cypress-all": "npx cypress run --spec 'cypress/tests/**/*.spec.ts'
--config baseUrl=http://localhost:8082";,
"prepare": "cd ../ && husky install ./ui/.husky",
- "i18n:extract": "ngx-translate-extract --input ./src --input ./projects
--output ./deployment/i18n/{en,de}.json -n --clean --format json",
+ "i18n:extract": "ngx-translate-extract --input ./src --input ./projects
--output ./deployment/i18n/{en,de}.json -n --clean --format json && node
scripts/sort-translations.js deployment/i18n/en.json && node
scripts/sort-translations.js deployment/i18n/de.json",
Review Comment:
The command chains multiple operations with && but doesn't provide clear
error messages if any of the sort operations fail. If sorting one file fails,
the subsequent files won't be sorted and the error may not be immediately
clear. Consider adding explicit error handling or logging between each sort
operation.
```suggestion
"i18n:extract": "ngx-translate-extract --input ./src --input ./projects
--output ./deployment/i18n/{en,de}.json -n --clean --format json && echo
\"Sorting en translations\" && node scripts/sort-translations.js
deployment/i18n/en.json && echo \"Sorting de translations\" && node
scripts/sort-translations.js deployment/i18n/de.json",
```
##########
ui/scripts/sort-translations.js:
##########
@@ -0,0 +1,18 @@
+const fs = require('fs');
+
+const file = process.argv[2];
+if (!file) {
+ console.error('Usage: node sort-translations.js <file>');
+ process.exit(1);
+}
+
+const json = JSON.parse(fs.readFileSync(file, 'utf8'));
+
+const sorted = Object.keys(json)
+ .sort((a, b) => a.localeCompare(b))
Review Comment:
The localeCompare method is being called with default options, which may
produce different sorting results depending on the system's locale. For
deterministic sorting across different environments, consider using
localeCompare with explicit options such as 'en' locale and numeric: false to
ensure consistent behavior.
```suggestion
.sort((a, b) => a.localeCompare(b, 'en', { numeric: false }))
```
##########
ui/scripts/sort-translations.js:
##########
@@ -0,0 +1,18 @@
+const fs = require('fs');
+
+const file = process.argv[2];
+if (!file) {
+ console.error('Usage: node sort-translations.js <file>');
+ process.exit(1);
+}
+
Review Comment:
The script doesn't check if the file exists before attempting to read it. If
the file doesn't exist, fs.readFileSync will throw an error that isn't caught,
causing the entire npm script chain to fail. Consider adding a file existence
check before reading.
```suggestion
if (!fs.existsSync(file)) {
console.error(`File not found: ${file}`);
process.exit(1);
}
```
##########
ui/scripts/sort-translations.js:
##########
@@ -0,0 +1,18 @@
+const fs = require('fs');
+
+const file = process.argv[2];
+if (!file) {
+ console.error('Usage: node sort-translations.js <file>');
+ process.exit(1);
+}
+
+const json = JSON.parse(fs.readFileSync(file, 'utf8'));
+
+const sorted = Object.keys(json)
+ .sort((a, b) => a.localeCompare(b))
+ .reduce((acc, key) => {
+ acc[key] = json[key];
+ return acc;
+ }, {});
+
+fs.writeFileSync(file, JSON.stringify(sorted, null, 2) + '\n');
Review Comment:
The script doesn't handle write errors. If the file write fails (e.g., due
to permission issues or disk space), fs.writeFileSync will throw an error that
isn't caught. Consider adding error handling for file write operations to
provide clear feedback when the operation fails.
```suggestion
try {
fs.writeFileSync(file, JSON.stringify(sorted, null, 2) + '\n');
} catch (err) {
console.error(`Failed to write sorted translations to "${file}":`, err
&& err.message ? err.message : err);
process.exit(1);
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
