Skip to content

Avoid reading each existing POT file twice during extraction - #448

Merged
whatyouhide merged 1 commit into
elixir-gettext:mainfrom
oliver-kriska:avoid-duplicate-pot-read
Oct 27, 2026
Merged

whatyouhide merged 1 commit into
elixir-gettext:mainfrom
oliver-kriska:avoid-duplicate-pot-read

Conversation

@oliver-kriska

Copy link
Copy Markdown
Contributor

Avoid a second disk read of each existing POT file during extraction by parsing the contents already read by File.read!/1. Expo.PO.parse_file!/2 delegates to the same string parser after reading the file; preserving file: path retains filename metadata and error attribution.

Split from #445 as requested, based directly on the latest upstream main. This is independent of the plural-info and interpolation-cache changes. It removes one read per POT file, not a second parse; no meaningful end-to-end speedup is claimed.

Verified with compilation using --warnings-as-errors, formatting, and the existing full suite: 193 checks (188 tests and 5 doctests) pass on Elixir 1.20.4/OTP 29 and 1.18.4/OTP 27. The three split changes also combine without conflicts and pass the full suite together. The earlier real-catalog validation remains documented in #445; it was not rerun for this split.

`read_contents_and_parse/1` read the file with `File.read!/1` and then called
`PO.parse_file!/2`, which reads the very same file again internally. Pass the
contents we already have to `PO.parse_string!/2` instead.

`PO.parse_file!(path, file: path)` is defined as `File.read` followed by
`parse_string(contents, file: path)`, so the parsed result is identical; the
existing `File.read!/1` still raises on a missing file. This removes one disk
read per existing POT file on every `mix gettext.extract`.
@whatyouhide

Copy link
Copy Markdown
Contributor

Nice thank you @oliver-kriska 🫶

@whatyouhide
whatyouhide merged commit b2c62ff into elixir-gettext:main Oct 27, 2026
2 checks passed
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.

3 participants