DevTools: Update presubmit script to remove duplicate grdp messages
Due to the merge timing of patches, sometimes duplicate grdp messages
can be added and co-exist*. This patch updates the presubmit script to
automatically delete duplicate grdp messages.
- Keep the winning grdp message (i.e. the one that corresponds to the
frontend message that comes first based on its file name when sorted).
- If none of these duplicate messages should be kept, delete them all.
- If none of these duplicate messages should be kept AND the same message
needs to be created in a different grdp file, preserve the longer
description.
In addition, this patch removes unnecessary arguments such as isDebug and
file-level dictionary structures.
*: https://crrev.com/c/1580199 and https://crrev.com/c/1633989 both added
a grdp message for "Image from %s".
Bug: 941561
Change-Id: Ic37f1fc410361eabd0ba710ece73a76d78eb8d34
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1656070
Reviewed-by: Joel Einbinder <einbinder@chromium.org>
Commit-Queue: Mandy Chen <mandy.chen@microsoft.com>
Cr-Original-Commit-Position: refs/heads/master@{#677799}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: a0dbe54c77da415cfc79ee99bc0cd9e590ffb6ff
diff --git a/scripts/localization_utils/check_localized_strings.js b/scripts/localization_utils/check_localized_strings.js
index 32e1dde..117b2fa 100644
--- a/scripts/localization_utils/check_localized_strings.js
+++ b/scripts/localization_utils/check_localized_strings.js
@@ -8,10 +8,7 @@
* files and report error if present.
*/
-const fs = require('fs');
const path = require('path');
-const {promisify} = require('util');
-const writeFileAsync = promisify(fs.writeFile);
const localizationUtils = require('./localization_utils');
const escodegen = localizationUtils.escodegen;
const esprimaTypes = localizationUtils.esprimaTypes;
@@ -41,10 +38,10 @@
// Format
// {
-// IDS_KEY => {
+// IDS_KEY => a list of {
// actualIDSKey: string, // the IDS key in the message tag
// description: string,
-// filepath: string,
+// grdpPath: string,
// location: {
// start: {
// line: number
@@ -60,7 +57,7 @@
const devtoolsFrontendPath = path.resolve(__dirname, '..', '..', 'front_end');
-async function parseLocalizableResourceMaps(isDebug) {
+async function parseLocalizableResourceMaps() {
const grdpToFiles = new Map();
const dirs = await localizationUtils.getChildDirectoriesFromDirectory(devtoolsFrontendPath);
const grdpToFilesPromises = dirs.map(dir => {
@@ -70,12 +67,15 @@
});
await Promise.all(grdpToFilesPromises);
- const promises = [parseIDSKeys(localizationUtils.GRD_PATH, isDebug)];
+ const promises = [];
for (const [grdpPath, files] of grdpToFiles) {
files.forEach(file => fileToGRDPMap.set(file, grdpPath));
- promises.push(parseLocalizableStrings(files, isDebug));
+ promises.push(parseLocalizableStrings(files));
}
await Promise.all(promises);
+ // Parse grd(p) files after frontend strings are processed so we know
+ // what to add or remove based on frontend strings
+ await parseIDSKeys(localizationUtils.GRD_PATH);
}
/**
@@ -83,11 +83,9 @@
* Common.UIStringFormat, UI.formatLocalized or ls``) from devtools frontend files.
*/
-async function parseLocalizableStrings(devtoolsFiles, isDebug) {
+async function parseLocalizableStrings(devtoolsFiles) {
const promises = devtoolsFiles.map(filePath => parseLocalizableStringsFromFile(filePath));
await Promise.all(promises);
- if (isDebug)
- await writeFileAsync(path.resolve(__dirname, 'localizable_strings.json'), JSON.stringify(frontendStrings));
}
async function parseLocalizableStringsFromFile(filePath) {
@@ -225,12 +223,10 @@
* devtools frontend grdp files.
*/
-async function parseIDSKeys(grdFilePath, isDebug) {
+async function parseIDSKeys(grdFilePath) {
// NOTE: this function assumes that no <message> tags are present in the parent
const grdpFilePaths = await parseGRDFile(grdFilePath);
await parseGRDPFiles(grdpFilePaths);
- if (isDebug)
- await writeFileAsync(path.resolve(__dirname, 'IDS_Keys.json'), JSON.stringify(IDSkeys));
}
async function parseGRDFile(grdFilePath) {
@@ -317,17 +313,24 @@
message = localizationUtils.sanitizeStringIntoFrontendFormat(message);
const ids = localizationUtils.getIDSKey(message);
- IDSkeys.set(ids, {actualIDSKey, grdpPath: filePath, location: {start: {line}, end: {line}}, description});
+ addMessage(ids, actualIDSKey, filePath, line, description);
}
}
+function addMessage(expectedIDSKey, actualIDSKey, grdpPath, line, description) {
+ if (!IDSkeys.has(expectedIDSKey))
+ IDSkeys.set(expectedIDSKey, []);
+
+ IDSkeys.get(expectedIDSKey).push({actualIDSKey, grdpPath, location: {start: {line}, end: {line}}, description});
+}
+
/**
* The following functions compare frontend localizable strings
* with grdp <message>s and report error of resources to add,
* remove or modify.
*/
-async function getAndReportResourcesToAdd(frontendStrings, IDSkeys) {
- const keysToAddToGRD = getDifference(IDSkeys, frontendStrings);
+async function getAndReportResourcesToAdd() {
+ const keysToAddToGRD = getMessagesToAdd();
if (keysToAddToGRD.size === 0)
return;
@@ -350,18 +353,19 @@
return errorStr;
}
-function getAndReportResourcesToRemove(frontendStrings, IDSkeys) {
- const keysToRemoveFromGRD = getDifference(frontendStrings, IDSkeys);
+function getAndReportResourcesToRemove() {
+ const keysToRemoveFromGRD = getMessagesToRemove();
if (keysToRemoveFromGRD.size === 0)
return;
let errorStr =
'\nThe message(s) associated with the following IDS key(s) should be removed from its GRD/GRDP file(s):\n';
// Example error message:
- // third_party/blink/renderer/devtools/front_end/help/help_strings.grdp Line 18: IDS_DEVTOOLS_7d0ee6fed10d3d4e5c9ee496729ab519
- for (const [key, keyObj] of keysToRemoveFromGRD) {
- errorStr += `${localizationUtils.getRelativeFilePathFromSrc(keyObj.grdpPath)}${
- localizationUtils.getLocationMessage(keyObj.location)}: ${key}\n\n`;
+ // third_party/blink/renderer/devtools/front_end/accessibility/accessibility_strings.grdp Line 300: IDS_DEVTOOLS_c9bbad3047af039c14d0e7ec957bb867
+ for (const [ids, messages] of keysToRemoveFromGRD) {
+ messages.forEach(
+ message => errorStr += `${localizationUtils.getRelativeFilePathFromSrc(message.grdpPath)}${
+ localizationUtils.getLocationMessage(message.location)}: ${ids}\n\n`);
}
return errorStr;
}
@@ -374,36 +378,96 @@
let errorStr = '\nThe following GRD/GRDP message(s) do not have the correct IDS key.\n';
errorStr += 'Please update the key(s) by changing the "name" value.\n\n';
- for (const [expectedIDSKey, message] of messagesToModify) {
- errorStr += `${localizationUtils.getRelativeFilePathFromSrc(message.grdpPath)}${
- localizationUtils.getLocationMessage(message.location)}:\n`;
- errorStr += `${message.actualIDSKey} --> ${expectedIDSKey}\n\n`;
+ for (const [expectedIDSKey, messages] of messagesToModify) {
+ messages.forEach(
+ message => errorStr += `${localizationUtils.getRelativeFilePathFromSrc(message.grdpPath)}${
+ localizationUtils.getLocationMessage(
+ message.location)}:\n${message.actualIDSKey} --> ${expectedIDSKey}\n\n`);
}
return errorStr;
}
-/**
- * Output a Map containing sorted entries that are in @comparison but not @reference,
- * or entries that are in both but belong to different grdp files.
- */
-function getDifference(reference, comparison) {
+function getMessagesToAdd() {
+ // If a message with ids key exists in grdpPath
+ function messageExists(ids, grdpPath) {
+ const messages = IDSkeys.get(ids);
+ return messages.some(message => message.grdpPath === grdpPath);
+ }
+
const difference = [];
- for (const [key, value] of comparison) {
- if (!reference.has(key) || reference.get(key).grdpPath !== value.grdpPath)
- difference.push([key, value]);
+ for (const [ids, frontendString] of frontendStrings) {
+ if (!IDSkeys.has(ids) || !messageExists(ids, frontendString.grdpPath))
+ difference.push([ids, frontendString]);
}
return new Map(difference.sort());
}
+// Return a map from the expected IDS key to a list of messages
+// whose actual IDS keys need to be modified.
function getIDSKeysToModify() {
const messagesToModify = new Map();
- for (const [expectedIDSKey, message] of IDSkeys) {
- if (expectedIDSKey !== message.actualIDSKey)
- messagesToModify.set(expectedIDSKey, message);
+ for (const [expectedIDSKey, messages] of IDSkeys) {
+ for (const message of messages) {
+ if (expectedIDSKey !== message.actualIDSKey) {
+ if (messagesToModify.has(expectedIDSKey))
+ messagesToModify.get(expectedIDSKey).push(message);
+ else
+ messagesToModify.set(expectedIDSKey, [message]);
+ }
+ }
}
return messagesToModify;
}
+function getMessagesToRemove() {
+ const difference = new Map();
+ for (const [ids, messages] of IDSkeys) {
+ if (!frontendStrings.has(ids)) {
+ difference.set(ids, messages);
+ continue;
+ }
+
+ const expectedGrdpPath = frontendStrings.get(ids).grdpPath;
+ const messagesInGrdp = [];
+ const messagesToRemove = [];
+ messages.forEach(message => {
+ if (message.grdpPath !== expectedGrdpPath)
+ messagesToRemove.push(message);
+ else
+ messagesInGrdp.push(message);
+ });
+
+ if (messagesToRemove.length === 0 && messagesInGrdp.length === 1)
+ continue;
+
+ if (messagesInGrdp.length > 1) {
+ // If there are more than one messages with ids in the
+ // expected grdp file, keep one with the longest
+ // description and delete all the other messages
+ const longestDescription = getLongestDescription(messagesInGrdp);
+ let foundMessageToKeep = false;
+ for (const message of messagesInGrdp) {
+ if (message.description === longestDescription && !foundMessageToKeep) {
+ foundMessageToKeep = true;
+ continue;
+ }
+ messagesToRemove.push(message);
+ }
+ }
+ difference.set(ids, messagesToRemove);
+ }
+ return difference;
+}
+
+function getLongestDescription(messages) {
+ let longestDescription = '';
+ messages.forEach(message => {
+ if (message.description.length > longestDescription.length)
+ longestDescription = message.description;
+ });
+ return longestDescription;
+}
+
module.exports = {
frontendStrings,
IDSkeys,
@@ -411,6 +475,8 @@
getAndReportIDSKeysToModify,
getAndReportResourcesToAdd,
getAndReportResourcesToRemove,
- getDifference,
- getIDSKeysToModify
+ getIDSKeysToModify,
+ getLongestDescription,
+ getMessagesToAdd,
+ getMessagesToRemove,
};