feat(geocode): support "%dyncols:" for the opencage subcommands#3878
Merged
Conversation
The opencage/opencagenow subcommands previously rejected the special
"%dyncols:" --formatstr option. They now support it, dynamically adding
one output column per "{col_name:key}" pair where key is an OpenCage
dynamic field key (formatted, lat, lng, confidence, components.* or
annotations.*).
Unlike opencage_lookup (which caches a single formatted string keyed by
formatstr), the new opencage_lookup_dyncols caches the raw first result
as JSON, so the cache key is independent of the requested columns and
re-runs/duplicate queries still hit the cache.
- run(): drop the opencage "%dyncols:" rejection (the --new-column +
"%dyncols:" conflict check still applies)
- run_opencage(): parse & validate "%dyncols:" before any API call,
append dyncols headers, and keep the input row intact while appending
geocoded field values per row
- add is_valid_opencage_dyncol() and opencage_lookup_dyncols()
- USAGE & regenerated docs/help/geocode.md updated accordingly
- tests: 2 CI arg-validation tests + 1 ignored live-API test
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 7 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
- opencage_lookup_dyncols: use a NUL byte in the dyncols cache key's formatstr slot so it can never collide with opencage_lookup's key space (a --formatstr is a CLI arg and cannot contain a NUL) - run_opencage: reject malformed "%dyncols:" pairs (missing colon separator) and empty column names instead of silently dropping them - tests: add CI coverage for the empty-pairs, malformed-pair and empty-col-name error paths Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR extends the geocode opencage / geocode opencagenow subcommands to support the special --formatstr value %dyncols:, aligning OpenCage behavior with the other geocode subcommands that can dynamically append multiple output columns.
Changes:
- Removed the prior rejection of
%dyncols:for OpenCage subcommands and documented the new capability in command help. - Implemented
%dyncols:parsing/validation for OpenCage, row output appending, and a new cache path that stores the raw first OpenCage result JSON for flexible field extraction. - Added CI-runnable argument-validation tests plus an ignored live API round-trip test for dyncols output.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
src/cmd/geocode.rs |
Adds OpenCage %dyncols: parsing/validation, dyncols row writing behavior, and JSON-based caching for multi-column extraction. |
tests/test_geocode.rs |
Adds argument-validation tests for OpenCage dyncols and an ignored live API dyncols integration test. |
docs/help/geocode.md |
Regenerates help text to document %dyncols: support for OpenCage. |
- is_valid_opencage_dyncol(): require a non-empty suffix after the components./annotations. prefix so a bare prefix fails fast with the "Invalid '%dyncols:' key" error instead of silently emitting an empty column. - opencage_lookup_dyncols(): on the (unlikely) event JSON serialization of a geocoded result fails, skip caching and extract column values from the live result, instead of unwrap_or_default() caching an empty string that later decodes as a bogus, permanent zero-result. - geocode_opencage_dyncols test: add Workdir::read_stdout_on_success so the command runs once (asserting success) instead of twice, where the second run could fail yet still emit parseable CSV. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
address roborev job 2323: the non-empty-suffix validation for components./annotations. dyncols keys had no CI coverage. Add geocode_opencage_dyncols_bare_prefix_rejected asserting a bare prefix fails with the "Invalid '%dyncols:' key" error. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
address roborev job 2324: the previous test combined components. and annotations. in one formatstr, but validation short-circuits on the first invalid key so the annotations. branch was never reached. Loop over both prefixes as separate command invocations. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The
opencage/opencagenowgeocode subcommands previously rejected the special%dyncols:--formatstroption (("%dyncols:" is not supported by opencage.)). They now support it — just likesuggest/reverse/iplookup— dynamically adding one output column per{col_name:key}pair.$ qsv geocode opencage address -f '%dyncols: {city:components.city}, {pc:components.postcode}' file.csvkeyis any OpenCage dynamic field key:formatted,lat,lng,confidence,components.<name>orannotations.<dotted.path>.Details
run()— dropped the opencage%dyncols:rejection. The general--new-column+%dyncols:conflict check still applies.run_opencage()— parses & validates the%dyncols:formatstr before any API call (fails fast), appends one output column per pair, and in the row loop keeps the input row intact while appending the geocoded field values.is_valid_opencage_dyncol()— new helper; a key is valid if it'sformatted/lat/lng/confidenceor starts withcomponents./annotations.(mirrors whatopencage_field_valueresolves).opencage_lookup_dyncols()— new caching lookup. Unlikeopencage_lookup(which caches a single formatted string keyed by formatstr), this caches the raw first result as JSON, so the cache key is independent of which columns were requested — re-runs and duplicate queries still hit the cache.docs/help/geocode.mdregenerated viaqsv --generate-help-md.Tests
%dyncols:key rejected;%dyncols:+--new-columnrejected).#[ignore]'d live-API round-trip test (geocode_opencage_dyncols).cargo check,cargo clippy,cargo +nightly fmtall clean.cargo test geocode_opencage— 4 CI tests pass, 6 live tests ignored as designed.Note
The live round-trip test was not run here (no
QSV_OPENCAGE_API_KEYavailable). Run it with a key to confirm end-to-end against the real API.🤖 Generated with Claude Code