From c7a9f4d73f30d6bc4c032ca1058a4452ae126d60 Mon Sep 17 00:00:00 2001 From: TheWitness Date: Mon, 5 Oct 2026 12:29:06 -0400 Subject: [PATCH 1/2] ci: gate cacti.pot updates on i18n diff instead of regenerating The "Verify translation template is up to date" step regenerated locales/po/cacti.pot with locales/build_gettext.sh and compared it (ignoring POT-Creation-Date). That couples the check to an exact gettext/xgettext toolchain and fails on unrelated version drift. Replace it with tests/bin/check-i18n-pot.php, which inspects the pull request diff: if any changed line adds, removes, or edits a Cacti i18n call (__(), __n(), __esc(), ... the same keywords build_gettext.sh feeds xgettext) in a file that feeds the template, then locales/po/cacti.pot must also be part of the pull request. The check fails only when that pot update is missing. The step now runs only on pull_request events and diffs against the PR base SHA, mirroring the existing patch-coverage.php approach. --- .github/workflows/plugin-ci-workflow.yml | 15 +- tests/bin/check-i18n-pot.php | 263 +++++++++++++++++++++++ 2 files changed, 268 insertions(+), 10 deletions(-) create mode 100644 tests/bin/check-i18n-pot.php diff --git a/.github/workflows/plugin-ci-workflow.yml b/.github/workflows/plugin-ci-workflow.yml index e55a577..e9dd76c 100644 --- a/.github/workflows/plugin-ci-workflow.yml +++ b/.github/workflows/plugin-ci-workflow.yml @@ -178,18 +178,13 @@ jobs: find . -path './vendor' -prune -o -type f -name '*.php' -print0 | xargs -0 -r -n1 php -l - name: Verify translation template is up to date + if: github.event_name == 'pull_request' + env: + BASE_REF: ${{ github.event.pull_request.base.sha }} run: | cd ${{ github.workspace }}/cacti/plugins/reportit - sudo chmod +x locales/build_gettext.sh - sudo ./locales/build_gettext.sh - grep -v '^"POT-Creation-Date:' locales/po/cacti.pot > /tmp/cacti.pot.new - git checkout -- locales/ - grep -v '^"POT-Creation-Date:' locales/po/cacti.pot > /tmp/cacti.pot.old - if ! cmp -s /tmp/cacti.pot.old /tmp/cacti.pot.new; then - echo "locales/po/cacti.pot is out of date. Run locales/build_gettext.sh and commit the updated locales/po/cacti.pot." - diff -u /tmp/cacti.pot.old /tmp/cacti.pot.new || true - exit 1 - fi + git config --global --add safe.directory ${{ github.workspace }}/cacti/plugins/reportit + php tests/bin/check-i18n-pot.php "$BASE_REF" - name: Set expected Cacti version for unit tests run: echo -n "${{ env.CACTI }}" | sudo tee ${{ github.workspace }}/cacti/plugins/reportit/tests/.cacti-version > /dev/null diff --git a/tests/bin/check-i18n-pot.php b/tests/bin/check-i18n-pot.php new file mode 100644 index 0000000..183db00 --- /dev/null +++ b/tests/bin/check-i18n-pot.php @@ -0,0 +1,263 @@ + + * + * Exits 1 when a required pot update is missing, 2 on bad input, 0 otherwise. + */ + +/* + * The gettext keywords build_gettext.sh passes to xgettext. A changed line is + * only interesting when it contains one of these calls. The negative lookbehind + * keeps PHP magic methods such as __construct()/__toString() out of the match. + */ +$i18n_pattern = '/(?, removed: array, files: array} + */ +function collect_i18n_changes($range, $i18n_pattern) { + $command = 'git diff --no-ext-diff --unified=0 --no-color ' . $range . ' -- "*.php"'; + $output = []; + $status = 0; + + exec($command, $output, $status); + + if ($status !== 0) { + fwrite(STDERR, "git diff failed\n"); + + exit(2); + } + + $added = []; + $removed = []; + $files = []; + $old_file = null; + $new_file = null; + + foreach ($output as $line) { + if (strncmp($line, '--- ', 4) === 0) { + $old_file = diff_path(substr($line, 4)); + + continue; + } + + if (strncmp($line, '+++ ', 4) === 0) { + $new_file = diff_path(substr($line, 4)); + + continue; + } + + if ($line === '' || $line[0] !== '+' && $line[0] !== '-') { + continue; + } + + if (strncmp($line, '+++', 3) === 0 || strncmp($line, '---', 3) === 0) { + continue; + } + + $added_line = ($line[0] === '+'); + $file = $added_line ? $new_file : $old_file; + + if ($file === null || !feeds_template($file)) { + continue; + } + + $content = substr($line, 1); + + if (!preg_match($i18n_pattern, $content)) { + continue; + } + + $files[$file] = true; + + if ($added_line) { + $added[] = normalise_line($content); + } else { + $removed[] = normalise_line($content); + } + } + + return ['added' => $added, 'removed' => $removed, 'files' => $files]; +} + +/** + * Turn a diff header path ("a/foo.php", "b/foo.php" or "/dev/null") into a plain + * repository-relative path, or null when the side does not exist. + * + * @param string $raw Path portion following the "--- "/"+++ " marker. + * + * @return string|null Repository-relative path, or null for /dev/null. + */ +function diff_path($raw) { + $raw = trim($raw); + + if ($raw === '/dev/null') { + return null; + } + + if (strncmp($raw, 'a/', 2) === 0 || strncmp($raw, 'b/', 2) === 0) { + $raw = substr($raw, 2); + } + + return $raw; +} + +/** + * Whether a path is scanned by build_gettext.sh, i.e. a PHP file no more than + * one directory below the plugin root. + * + * @param string $file Repository-relative path. + * + * @return bool + */ +function feeds_template($file) { + if (substr($file, -4) !== '.php') { + return false; + } + + return substr_count($file, '/') <= 1; +} + +/** + * Whether the pull request already touches the gettext template. + * + * @param string $range Git diff range expression. + * @param string $pot_path Repository-relative path of cacti.pot. + * + * @return bool + */ +function pot_updated($range, $pot_path) { + $command = 'git diff --no-ext-diff --name-only --no-color ' . $range; + $output = []; + $status = 0; + + exec($command, $output, $status); + + if ($status !== 0) { + fwrite(STDERR, "git diff failed\n"); + + exit(2); + } + + foreach ($output as $name) { + if (trim($name) === $pot_path) { + return true; + } + } + + return false; +} + +$changes = collect_i18n_changes($range, $i18n_pattern); + +sort($changes['added']); +sort($changes['removed']); + +$i18n_changed = ($changes['added'] !== $changes['removed']); + +if (!$i18n_changed) { + fwrite(STDOUT, "No translatable-string changes detected in the diff; locales/po/cacti.pot update not required.\n"); + + exit(0); +} + +if (pot_updated($range, $pot_path)) { + fwrite(STDOUT, "Translatable strings changed and locales/po/cacti.pot is included in the pull request. OK.\n"); + + exit(0); +} + +$files = array_keys($changes['files']); +sort($files); + +fwrite(STDERR, "This pull request changes i18n strings but does not update locales/po/cacti.pot.\n"); +fwrite(STDERR, "Run locales/build_gettext.sh and commit the regenerated locales/po/cacti.pot.\n"); +fwrite(STDERR, "\n"); +fwrite(STDERR, "Files with changed i18n calls:\n"); + +foreach ($files as $file) { + fwrite(STDERR, " - $file\n"); +} + +exit(1); From 080e064a6c826702fae7701ad49b0903b07c346f Mon Sep 17 00:00:00 2001 From: TheWitness Date: Mon, 5 Oct 2026 13:41:25 -0400 Subject: [PATCH 2/2] ci: compare i18n strings via tokenizer instead of diff lines Address the reviewer note that the pot gate had false-positive and false-negative cases. The previous check scanned the unified diff line by line and compared whole normalised lines. That produced: - false positives: changing unrelated code on a line that also holds an i18n call (e.g. a width argument next to __()) demanded a pot update even though the translatable string was unchanged; and - false negatives: a multi-line i18n call whose string literal sits on its own line was missed, so changing the string slipped through. Rebuild the comparison from the tokens instead. For the merge-base and for HEAD, tokenize every template-feeding PHP file (the same find . -maxdepth 2 set build_gettext.sh scans), find the Cacti i18n helper calls, and collect the literal string arguments that actually form each pot entry (msgctxt/msgid/plural, per the xgettext -k spec). A call only contributes when those positions are constant strings, mirroring xgettext. If the extracted set differs between base and HEAD, locales/po/cacti.pot must be part of the pull request. Because it reads whole files rather than diff lines, surrounding-code edits and quote-style-only changes no longer trigger it, and multi-line calls are handled correctly. --- tests/bin/check-i18n-pot.php | 574 ++++++++++++++++++++++++++++------- 1 file changed, 472 insertions(+), 102 deletions(-) diff --git a/tests/bin/check-i18n-pot.php b/tests/bin/check-i18n-pot.php index 183db00..1e45fb6 100644 --- a/tests/bin/check-i18n-pot.php +++ b/tests/bin/check-i18n-pot.php @@ -21,9 +21,17 @@ * locales/po/cacti.pot is produced by locales/build_gettext.sh, which runs * xgettext over `find . -maxdepth 2 -name '*.php'` extracting the Cacti i18n * helpers (__(), __n(), __esc(), ...). Rather than regenerating the template - * and comparing timestamps, this inspects the branch diff: if any changed line - * adds, removes, or edits one of those i18n calls in a file that feeds the - * template, then locales/po/cacti.pot must also be part of the pull request. + * and comparing timestamps (which couples CI to one exact gettext toolchain), + * this reconstructs the set of translatable strings xgettext would extract at + * the merge-base and at HEAD and compares them. If that set changed, then + * locales/po/cacti.pot must also be part of the pull request. + * + * The strings are recovered with PHP's own tokenizer rather than a line regex, + * so that: + * - a change to surrounding code on a line that also holds an i18n call does + * not look like a string change (no false positive), and + * - a multi-line i18n call whose literal sits on its own line is still seen + * (no false negative). * * It is not this check's job to prove the template is byte-correct, only that * it was regenerated and committed when the translatable strings moved. @@ -34,11 +42,28 @@ */ /* - * The gettext keywords build_gettext.sh passes to xgettext. A changed line is - * only interesting when it contains one of these calls. The negative lookbehind - * keeps PHP magic methods such as __construct()/__toString() out of the match. + * The gettext keywords build_gettext.sh passes to xgettext, mapped to the + * 1-based argument positions that form the pot entry (msgctxt/msgid/plural): + * + * -k__gettext -k__ -k__n:1,2 -k__x:1c,2 -k__xn:1c,2,3 + * -k__esc -k__esc_n:1,2 -k__esc_x:1c,2 -k__esc_xn:1c,2,3 -k__date + * + * A call only contributes an entry when every listed position is a literal + * string (optionally a concatenation of literals), mirroring xgettext, which + * silently ignores calls whose keyword argument is not a constant string. */ -$i18n_pattern = '/(? [1], + '__' => [1], + '__n' => [1, 2], + '__x' => [1, 2], + '__xn' => [1, 2, 3], + '__esc' => [1], + '__esc_n' => [1, 2], + '__esc_x' => [1, 2], + '__esc_xn' => [1, 2, 3], + '__date' => [1], +]; /* Repository-relative path of the template the plugin must keep in sync. */ $pot_path = 'locales/po/cacti.pot'; @@ -70,145 +95,495 @@ exec('git fetch --quiet --no-tags --depth=200 origin ' . escapeshellarg($base_ref), $ignore, $ignore_status); } -$range = escapeshellarg($base_ref) . '...HEAD'; +/* + * Compare HEAD against the point the branch diverged from the base so that + * unrelated commits landing on the base after branch-off are not attributed to + * this pull request. Fall back to the base ref itself if no merge-base exists + * (e.g. an unrelated-history or very shallow checkout). + */ +$base_commit = git_capture_line('git merge-base ' . escapeshellarg($base_ref) . ' HEAD'); + +if ($base_commit === null) { + $base_commit = $base_ref; +} + +$base_strings = collect_strings($base_commit, $i18n_keywords); +$head_strings = collect_strings('HEAD', $i18n_keywords); + +sort($base_strings); +sort($head_strings); + +if ($base_strings === $head_strings) { + fwrite(STDOUT, "No translatable-string changes detected; locales/po/cacti.pot update not required.\n"); + + exit(0); +} + +if (pot_updated($base_commit, $pot_path)) { + fwrite(STDOUT, "Translatable strings changed and locales/po/cacti.pot is included in the pull request. OK.\n"); + + exit(0); +} + +$added = array_values(array_diff($head_strings, $base_strings)); +$removed = array_values(array_diff($base_strings, $head_strings)); + +fwrite(STDERR, "This pull request changes i18n strings but does not update locales/po/cacti.pot.\n"); +fwrite(STDERR, "Run locales/build_gettext.sh and commit the regenerated locales/po/cacti.pot.\n"); +fwrite(STDERR, "\n"); + +report_strings('Added translatable strings', $added); +report_strings('Removed translatable strings', $removed); + +exit(1); /** - * Normalise a source line so that a pure re-indent or reflow of an i18n call - * does not read as a content change. + * Run a git command expected to print a single value and return it trimmed. * - * @param string $line Raw diff line with its leading +/- already removed. + * @param string $command Fully-escaped git command line. * - * @return string Whitespace-collapsed, trimmed line. + * @return string|null The first output line, or null when the command failed + * or produced nothing. */ -function normalise_line($line) { - return preg_replace('/\s+/', ' ', trim($line)); +function git_capture_line($command) { + $output = []; + $status = 0; + + exec($command . ' 2>/dev/null', $output, $status); + + if ($status !== 0 || !isset($output[0]) || trim($output[0]) === '') { + return null; + } + + return trim($output[0]); } /** - * Collect the i18n-bearing lines each side of the diff adds or removes, limited - * to files that actually feed the gettext template. + * Build the multiset of translatable strings xgettext would extract from the + * template-feeding PHP files at a given revision. + * + * @param string $ref Git revision to read the tree from. + * @param array $keywords Map of i18n keyword to contributing argument + * positions. * - * A file feeds the template when it is a PHP file no deeper than one directory - * below the plugin root, mirroring `find . -maxdepth 2 -name '*.php'`. + * @return array Canonical "keyword\x1farg...\x1farg" entries, one + * per qualifying i18n call. + */ +function collect_strings($ref, $keywords) { + $strings = []; + + foreach (template_files($ref) as $file) { + $code = git_show($ref, $file); + + if ($code === null) { + continue; + } + + foreach (extract_calls($code, $keywords) as $entry) { + $strings[] = $entry; + } + } + + return $strings; +} + +/** + * List the PHP files that feed build_gettext.sh at a revision, i.e. those no + * more than one directory below the plugin root (`find . -maxdepth 2`). * - * @param string $range Git diff range expression. - * @param string $i18n_pattern Regex matching the Cacti i18n helper calls. + * @param string $ref Git revision to read the tree from. * - * @return array{added: array, removed: array, files: array} + * @return array Repository-relative PHP paths. */ -function collect_i18n_changes($range, $i18n_pattern) { - $command = 'git diff --no-ext-diff --unified=0 --no-color ' . $range . ' -- "*.php"'; - $output = []; - $status = 0; +function template_files($ref) { + $output = []; + $status = 0; - exec($command, $output, $status); + exec('git ls-tree -r --name-only ' . escapeshellarg($ref), $output, $status); if ($status !== 0) { - fwrite(STDERR, "git diff failed\n"); + fwrite(STDERR, "git ls-tree failed for " . $ref . "\n"); exit(2); } - $added = []; - $removed = []; - $files = []; - $old_file = null; - $new_file = null; + $files = []; - foreach ($output as $line) { - if (strncmp($line, '--- ', 4) === 0) { - $old_file = diff_path(substr($line, 4)); + foreach ($output as $file) { + $file = trim($file); - continue; + if (feeds_template($file)) { + $files[] = $file; } + } + + return $files; +} - if (strncmp($line, '+++ ', 4) === 0) { - $new_file = diff_path(substr($line, 4)); +/** + * Whether a path is scanned by build_gettext.sh, i.e. a PHP file no more than + * one directory below the plugin root. + * + * @param string $file Repository-relative path. + * + * @return bool + */ +function feeds_template($file) { + if (substr($file, -4) !== '.php') { + return false; + } + + return substr_count($file, '/') <= 1; +} + +/** + * Read a file's contents at a revision. + * + * @param string $ref Git revision. + * @param string $file Repository-relative path. + * + * @return string|null File contents, or null when the file is absent there. + */ +function git_show($ref, $file) { + $descriptors = [ + 1 => ['pipe', 'w'], + 2 => ['pipe', 'w'], + ]; + + $process = proc_open('git show ' . escapeshellarg($ref . ':' . $file), $descriptors, $pipes); + + if (!is_resource($process)) { + return null; + } + + $code = stream_get_contents($pipes[1]); + fclose($pipes[1]); + fclose($pipes[2]); + $status = proc_close($process); + + if ($status !== 0) { + return null; + } + + return $code; +} + +/** + * Extract the translatable-string entries from PHP source using the tokenizer. + * + * @param string $code PHP source. + * @param array $keywords Map of i18n keyword to contributing argument + * positions. + * + * @return array Canonical entries for each qualifying i18n call. + */ +function extract_calls($code, $keywords) { + $tokens = @token_get_all($code); + $count = count($tokens); + $calls = []; + + for ($i = 0; $i < $count; $i++) { + $token = $tokens[$i]; + + if (!is_array($token) || $token[0] !== T_STRING || !isset($keywords[$token[1]])) { continue; } - if ($line === '' || $line[0] !== '+' && $line[0] !== '-') { + $prev = previous_significant($tokens, $i); + + if ($prev !== null && is_array($prev) && in_array($prev[0], call_disqualifiers(), true)) { continue; } - if (strncmp($line, '+++', 3) === 0 || strncmp($line, '---', 3) === 0) { + $open = next_significant_index($tokens, $i); + + if ($open === null || $tokens[$open] !== '(') { continue; } - $added_line = ($line[0] === '+'); - $file = $added_line ? $new_file : $old_file; + $args = parse_arguments($tokens, $open); + $parts = []; + $ok = true; - if ($file === null || !feeds_template($file)) { - continue; + foreach ($keywords[$token[1]] as $position) { + if (!isset($args[$position - 1])) { + $ok = false; + + break; + } + + $literal = literal_value($args[$position - 1]); + + if ($literal === null) { + $ok = false; + + break; + } + + $parts[] = $literal; + } + + if ($ok) { + $calls[] = $token[1] . "\x1f" . implode("\x1f", $parts); } + } - $content = substr($line, 1); + return $calls; +} - if (!preg_match($i18n_pattern, $content)) { - continue; +/** + * Token types that, immediately before a keyword, mean it is a method call or + * declaration rather than the i18n helper (e.g. $o->__(), C::__(), function __). + * + * @return array Token id constants. + */ +function call_disqualifiers() { + $ids = [T_OBJECT_OPERATOR, T_DOUBLE_COLON, T_FUNCTION, T_NEW]; + + if (defined('T_NULLSAFE_OBJECT_OPERATOR')) { + $ids[] = T_NULLSAFE_OBJECT_OPERATOR; + } + + return $ids; +} + +/** + * The significant token preceding a position (skipping whitespace/comments). + * + * @param array $tokens token_get_all() output. + * @param int $index Position to look back from. + * + * @return array|string|null The token, or null at the start of the stream. + */ +function previous_significant($tokens, $index) { + for ($i = $index - 1; $i >= 0; $i--) { + if (is_significant($tokens[$i])) { + return $tokens[$i]; + } + } + + return null; +} + +/** + * Index of the significant token following a position. + * + * @param array $tokens token_get_all() output. + * @param int $index Position to look forward from. + * + * @return int|null Index of the next significant token, or null at end. + */ +function next_significant_index($tokens, $index) { + $count = count($tokens); + + for ($i = $index + 1; $i < $count; $i++) { + if (is_significant($tokens[$i])) { + return $i; } + } + + return null; +} + +/** + * Whether a token carries code (not whitespace or a comment). + * + * @param array|string $token A token_get_all() element. + * + * @return bool + */ +function is_significant($token) { + if (!is_array($token)) { + return true; + } - $files[$file] = true; + return !in_array($token[0], [T_WHITESPACE, T_COMMENT, T_DOC_COMMENT], true); +} + +/** + * Split a call's top-level arguments, starting at the opening parenthesis. + * + * @param array $tokens token_get_all() output. + * @param int $open Index of the '(' that opens the argument list. + * + * @return array One token list per argument (empty list for none). + */ +function parse_arguments($tokens, $open) { + $count = count($tokens); + $depth = 0; + $args = []; + $current = []; + + for ($i = $open; $i < $count; $i++) { + $token = $tokens[$i]; + + if (!is_array($token)) { + if ($token === '(' || $token === '[' || $token === '{') { + $depth++; + + if ($depth === 1) { + continue; + } + } elseif ($token === ')' || $token === ']' || $token === '}') { + $depth--; + + if ($depth === 0) { + $args[] = $current; + + break; + } + } elseif ($token === ',' && $depth === 1) { + $args[] = $current; + $current = []; + + continue; + } + } - if ($added_line) { - $added[] = normalise_line($content); - } else { - $removed[] = normalise_line($content); + if ($depth >= 1) { + $current[] = $token; } } - return ['added' => $added, 'removed' => $removed, 'files' => $files]; + return $args; } /** - * Turn a diff header path ("a/foo.php", "b/foo.php" or "/dev/null") into a plain - * repository-relative path, or null when the side does not exist. + * Resolve an argument to its string value when it is a literal string, or a + * concatenation of literal strings, matching what xgettext can extract. * - * @param string $raw Path portion following the "--- "/"+++ " marker. + * @param array $tokens The argument's token list. * - * @return string|null Repository-relative path, or null for /dev/null. + * @return string|null The decoded string, or null when not a constant string. */ -function diff_path($raw) { - $raw = trim($raw); +function literal_value($tokens) { + $parts = []; + $expect_string = true; + + foreach ($tokens as $token) { + if (!is_significant($token)) { + continue; + } + + if ($expect_string) { + if (is_array($token) && $token[0] === T_CONSTANT_ENCAPSED_STRING) { + $parts[] = decode_string($token[1]); + $expect_string = false; + + continue; + } + + return null; + } + + if ($token === '.') { + $expect_string = true; + + continue; + } - if ($raw === '/dev/null') { return null; } - if (strncmp($raw, 'a/', 2) === 0 || strncmp($raw, 'b/', 2) === 0) { - $raw = substr($raw, 2); + if ($expect_string && $parts !== []) { + return null; } - return $raw; + if ($parts === []) { + return null; + } + + return implode('', $parts); } /** - * Whether a path is scanned by build_gettext.sh, i.e. a PHP file no more than - * one directory below the plugin root. + * Decode a single- or double-quoted PHP string literal to its runtime value so + * that a pure quote-style change is not mistaken for a content change. The + * tokenizer only emits T_CONSTANT_ENCAPSED_STRING for strings without + * interpolation, so no variable expansion is required here. * - * @param string $file Repository-relative path. + * @param string $raw The literal including its surrounding quotes. * - * @return bool + * @return string The decoded value. */ -function feeds_template($file) { - if (substr($file, -4) !== '.php') { - return false; +function decode_string($raw) { + if (strlen($raw) < 2) { + return $raw; } - return substr_count($file, '/') <= 1; + $quote = $raw[0]; + $inner = substr($raw, 1, -1); + + if ($quote === "'") { + return strtr($inner, ['\\\\' => '\\', "\\'" => "'"]); + } + + return preg_replace_callback( + '/\\\\(?:x[0-9A-Fa-f]{1,2}|[0-7]{1,3}|u\{[0-9A-Fa-f]+\}|.)/', + 'decode_double_quoted_escape', + $inner + ); +} + +/** + * Expand one backslash escape from a double-quoted string literal. + * + * @param array $match preg_replace_callback match; $match[0] is the escape. + * + * @return string The expanded character(s). + */ +function decode_double_quoted_escape($match) { + $escape = $match[0]; + $char = $escape[1]; + + $simple = [ + 'n' => "\n", + 't' => "\t", + 'r' => "\r", + 'v' => "\v", + 'f' => "\f", + 'e' => "\e", + '"' => '"', + '$' => '$', + '\\' => '\\', + ]; + + if (isset($simple[$char])) { + return $simple[$char]; + } + + if ($char === 'x') { + return chr(hexdec(substr($escape, 2))); + } + + if ($char === 'u') { + $code = hexdec(substr($escape, 3, -1)); + + if (function_exists('mb_chr')) { + return mb_chr($code, 'UTF-8'); + } + + return $escape; + } + + if ($char >= '0' && $char <= '7') { + return chr(octdec(substr($escape, 1)) & 0xFF); + } + + return $escape; } /** * Whether the pull request already touches the gettext template. * - * @param string $range Git diff range expression. - * @param string $pot_path Repository-relative path of cacti.pot. + * @param string $base_commit Commit to diff HEAD against. + * @param string $pot_path Repository-relative path of cacti.pot. * * @return bool */ -function pot_updated($range, $pot_path) { - $command = 'git diff --no-ext-diff --name-only --no-color ' . $range; +function pot_updated($base_commit, $pot_path) { + $command = 'git diff --no-ext-diff --name-only --no-color ' . escapeshellarg($base_commit) . ' HEAD'; $output = []; $status = 0; @@ -229,35 +604,30 @@ function pot_updated($range, $pot_path) { return false; } -$changes = collect_i18n_changes($range, $i18n_pattern); - -sort($changes['added']); -sort($changes['removed']); - -$i18n_changed = ($changes['added'] !== $changes['removed']); - -if (!$i18n_changed) { - fwrite(STDOUT, "No translatable-string changes detected in the diff; locales/po/cacti.pot update not required.\n"); - - exit(0); -} - -if (pot_updated($range, $pot_path)) { - fwrite(STDOUT, "Translatable strings changed and locales/po/cacti.pot is included in the pull request. OK.\n"); +/** + * Print a labelled, de-duplicated list of decoded i18n entries for diagnostics. + * + * @param string $label Human-readable heading. + * @param array $entries Canonical "keyword\x1farg..." entries. + * + * @return void + */ +function report_strings($label, $entries) { + if ($entries === []) { + return; + } - exit(0); -} + fwrite(STDERR, $label . ":\n"); -$files = array_keys($changes['files']); -sort($files); + foreach (array_unique($entries) as $entry) { + $fields = explode("\x1f", $entry); + $keyword = array_shift($fields); + $shown = array_map(function ($field) { + return '"' . $field . '"'; + }, $fields); -fwrite(STDERR, "This pull request changes i18n strings but does not update locales/po/cacti.pot.\n"); -fwrite(STDERR, "Run locales/build_gettext.sh and commit the regenerated locales/po/cacti.pot.\n"); -fwrite(STDERR, "\n"); -fwrite(STDERR, "Files with changed i18n calls:\n"); + fwrite(STDERR, ' - ' . $keyword . '(' . implode(', ', $shown) . ")\n"); + } -foreach ($files as $file) { - fwrite(STDERR, " - $file\n"); + fwrite(STDERR, "\n"); } - -exit(1);