Skip to content

Fix translation count and find precedence in the download step - #17

Open
SNO7E-G wants to merge 1 commit into
OnTheGoSystems:mainfrom
SNO7E-G:fix-download-translations
Open

Fix translation count and find precedence in the download step#17
SNO7E-G wants to merge 1 commit into
OnTheGoSystems:mainfrom
SNO7E-G:fix-download-translations

Conversation

@SNO7E-G

@SNO7E-G SNO7E-G commented Aug 15, 2026

Copy link
Copy Markdown
Contributor

Summary. Two correctness fixes in download_translations' extraction block.

Problem.

  1. moved_count was incremented inside a find … | while read pipeline, which runs the loop in a subshell — the parent counter never changed, so the "Moved N files" log always reported 0, even on a successful unpack.
  2. find … -type f -name "*.json" -o -name "*.po" … binds -type f only to the first -name (find's implicit -a is tighter than -o), so the other extensions matched entries of any type — a directory named like a match could enter the move loop.

Change. Feed the loop via process substitution (while … done < <(find …)) so moved_count and a new move_failed flag survive; group the -name alternatives with \( … \) so -type f applies to all (regular files only).
The success/failure contract is preserved exactly: a failed move logs a warning, sets move_failed, breaks; the post-loop check then logs the error, rm -rfs both temp paths, returns 1. Success cleans up and returns 0.

Risk. Localised to the extract/move block; no interface change. One cosmetic shift — the rare "mv succeeded then file vanished" sub-case now logs the generic "Failed to move" message rather than its own text (same detection, abort, cleanup).

Verified. bash -n clean; a harness mirroring the new block moves 3 real files (count = 3) while a directory named d.po and a .txt are skipped. This block runs only on a real HTTP-200 zip (no stubbed suite reaches it), so the harness is the achievable check without a live token. Independent review confirmed failure path + cleanup preserved and set -e-safe. Rebased cleanly onto v1.0.3; bug confirmed still present upstream at ~lines 2498–2530.

Two issues in download_translations' extraction step, same block:

- moved_count was incremented inside a `find ... | while read` pipeline, which
  runs in a subshell, so the parent counter never changed and the "Moved N
  files" log always reported 0. Feed the loop with process substitution so the
  count survives.
- `find -type f -name "*.json" -o -name "*.po" ...` bound -type f only to the
  first -name, so a directory named like a match could slip through. Group the
  -name alternatives with \( \) so -type f applies to all of them.

Success/failure and cleanup behaviour is preserved via a move_failed flag.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant