Skip to content

feat(ui): offer field names in arrays and in keys that take a field in the Script pane - #856

Merged
giswqs merged 2 commits into
opengeos:mainfrom
jgregoriorincon:feat/script-field-arrays
Oct 25, 2026
Merged

giswqs merged 2 commits into
opengeos:mainfrom
jgregoriorincon:feat/script-field-arrays

Conversation

@jgregoriorincon

@jgregoriorincon jgregoriorincon commented Oct 24, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

The Script pane now offers field names wherever a command's description gives a field as a key's type, not only for "field", and inside the key's array when the type is a list (#846):

  • Which keys: those whose type starts with field, [field or [numeric field (field, field|null, [fields], [field, …], field | [fields], [numeric fields]), in a command that takes a "layer", e.g. data.explore/data.similar "fields", share.locator "fields", report.new "groups"/"sort", data.convert "nameField", share.export3d "heightField", layer.geocode "address". Commands with no "layer" are left out, since their fields belong to another layer (data.relate's originKey/destinationKey, a report chart's category/value), as are [related fields] and [names].
  • Which fields: the line's layer's, as "field" offers them; only numeric ones where the type says numeric; for layer.geocode with a "table", that table's fields (Of::TableFields, with type and alias beside each).
  • In an array: a string right in a top-level key's array counts as that key's value (scan), but only list-typed field keys offer there: Run turns down an array for a layer, a tool, a tool's parameter or a single field, so nothing is offered for those, and value_of ignores array strings. Fields the array already lists, before or after the cursor, aren't offered again: the same field twice would do the same work twice.

Behaviour change: a "field" described as numeric field (layer.extrusion, analysis.interpolate, analysis.simulate) now offers only the numeric fields, as its description says. The user guide (Analysis › Script) describes the new values.

Testing

  • cargo xtask ci passes.
  • New test keys_that_take_fields_offer_them_in_arrays_too: numeric and plain lists, fields given before and after the cursor left out (the one being typed kept), nameField and address, data.relate and [related fields] left out, nested arrays, arrays for a single field, a layer, a tool or a tool's parameter, a layer given in an array, and layer.geocode's table (case aside, help and icon, a missing table, a table given as a number).
  • Manual, on the World Atlas sample: data.explore numeric fields, a chosen field not offered again (after and before the cursor), share.locator, report.new "sort", layer.extrusion numeric only, layer.geocode with a CSV table, nothing for data.relate's originKey, for "layer": [" or for "field": ["; labels.field unchanged.

Related issue

Closes #846

Summary by CodeRabbit

  • New Features
    • Script autocomplete now suggests fields for command parameters that accept one or more fields, including within arrays. Suggestions reflect the relevant layer or named table, exclude fields already selected in the same array, and respect numeric-field restrictions.
  • Documentation
    • The Script pane guide now explains field suggestions and array behavior, with examples for exploring data, creating reports, and geocoding.

@coderabbitai

coderabbitai Bot commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: 4ecccc34-d5d6-4e59-9172-8d144975f225
Commits

Reviewing files that changed from the base of the PR and between c281e2a and ee97b78.

Files selected for processing: 2
  • crates/ui-egui/src/scriptcomplete.rs
  • docs/user-guide/analysis.md

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.

Walkthrough

Script completion now offers fields for command parameters whose declared types accept fields. It supports field arrays, numeric-field filtering, duplicate exclusion, and field suggestions from a named table for layer.geocode.

Changes: Script field completion

Layer / File(s) Summary
Track array values
crates/ui-egui/src/scriptcomplete.rs
The scanner tracks array membership for values. Top-level value lookup and tool-parameter completion exclude array entries.
Offer field-valued completions
crates/ui-egui/src/scriptcomplete.rs
Parameter descriptions determine which keys accept fields. Suggestions support numeric filtering, exclude fields already in the array, and use either the line’s layer or the named table. Table fields also provide field help and icons.
Validate and document completion
crates/ui-egui/src/scriptcomplete.rs, docs/user-guide/analysis.md
Tests cover field suggestions, filtering, field sources, help, icons, and type parsing. The user guide adds examples for supported parameters.

Priority: Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Feature

Sequence Diagram(s)

sequenceDiagram
  participant ScriptPane
  participant Scan as scriptcomplete::scan
  participant Offer as scriptcomplete::offer_at
  participant Fields as Line layer or named table fields
  ScriptPane->>Scan: Parse cursor context and array membership
  Scan->>Offer: Provide key, value, and listed state
  Offer->>Fields: Resolve candidate fields
  Fields-->>Offer: Return field metadata
  Offer-->>ScriptPane: Return filtered suggestions
Loading

Possibly related issues

  • opengeos/RustGIS#662 — This PR extends Script pane field autocomplete, which is part of the parent issue’s autocomplete work.

Possibly related PRs

  • opengeos/RustGIS#827 — Adds Script pane field completion for gp.run parameters and shared table input functionality used by this change.
  • opengeos/RustGIS#771 — Adds the initial field completion from a line’s layer, which this change generalizes to more command parameters.
  • opengeos/RustGIS#743 — Provides the Quotes contract used when scanning quoted values in field arrays.

Suggested reviewers: giswqs

Merge Risk: Minimal · up to ee97b

The documented completion behavior matches the supported cases, and no actionable issue remains before merge.

Pre-merge checks: Passed 5
Check name Status Explanation
Title check Passed The title names the main change: field-name suggestions in the Script pane for keys that take a field type, including arrays. It matches the changeset in scriptcomplete.rs and the analysis.md docs. It…
Description check Passed The description follows the template. It has a Summary that explains what changed and why, a Testing section listing cargo xtask ci, a new test, and manual checks, and a Related issue section that clo…
Linked Issues check Passed Issue #846 requires field suggestions for field-typed values and strings in field-list arrays, using the line layer or the layer.geocode table, applying numeric filtering, preserving matching/displa…
Out of Scope Changes check Passed The reported changes are limited to Script pane completion logic, its focused automated test coverage, and user-guide documentation for issue #846. The Of::TableFields and ValueAt.listed changes s…
Docstring Coverage Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 15 functions across 1 files. (1 skipped: 1…
Full details: Title check

Explanation

The title names the main change: field-name suggestions in the Script pane for keys that take a field type, including arrays. It matches the changeset in scriptcomplete.rs and the analysis.md docs. It is long (86 characters) but still specific and accurate.

Full details: Description check

Explanation

The description follows the template. It has a Summary that explains what changed and why, a Testing section listing cargo xtask ci, a new test, and manual checks, and a Related issue section that closes #846. The summary is detailed and matches the objectives and the raw summary.

Full details: Linked Issues check

Explanation

Issue #846 requires field suggestions for field-typed values and strings in field-list arrays, using the line layer or the layer.geocode table, applying numeric filtering, preserving matching/display/insertion behavior, and excluding unrelated contexts. The PR summary reports implementation in crates/ui-egui/src/scriptcomplete.rs, including Of::TableFields, array tracking, duplicate suppression, numeric filtering, layer and table resolution, and the listed exclusions. The reported test covers these cases. The user guide documents the feature.

Full details: Out of Scope Changes check

Explanation

The reported changes are limited to Script pane completion logic, its focused automated test coverage, and user-guide documentation for issue #846. The Of::TableFields and ValueAt.listed changes support the completion behavior. No unrelated change is identified.

Full details: Docstring Coverage

Explanation

Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 15 functions across 1 files. (1 skipped: 1 unsupported.)

Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

A rabbit types a field name bright,
Suggestions hop into the light.
Arrays skip names already there,
Numeric fields get careful care.
Table fields join the list with flair.

Comment @coderabbitai help to get the list of available commands.

Comment thread crates/ui-egui/src/scriptcomplete.rs
@github-actions

Copy link
Copy Markdown
Contributor

Code review

The change is small and well tested. I found one medium-confidence concern and a few lower-confidence notes. I did not run the code or the tests.

Bugs

  • Medium confidence: field_type (scriptcomplete.rs:915) decides "is a list" with ty.contains('['). ty is the whole description after the first :, not just the type. A single-field key whose prose has brackets, such as field: the name [optional], would be treated as a list, so an array would be accepted for it and Run would turn that down. I posted this as an inline comment.
  • Low confidence: In the shared "field" arm, takes(session, &hint.id, "table") plus a "table" key on the line forces the table path. A command that has both "layer" and "table" and a "field" key will offer the table's fields even when the layer is the intended source. This only matters for such commands. I didn't find one, so it may be moot.
  • Low confidence: value_of(line, "table")? returns None for a table name containing an escape (\). The offer then silently disappears instead of falling back. This is a safe failure, just a small gap in what gets offered.

Security

  • Nothing found. The code only parses script text and reads the session. It has no unwrap or indexing, and it uses get() and ? throughout, as AGENTS.md requires.

Performance

  • Nothing significant. scan runs once more per offer, to build the "already given" list, on a single line of text.

Quality

  • The scanner's value and values tuples now carry three fields, (usize, String, bool) and (String, String, bool). A small named struct would read more clearly than positional tuples, and the (k, v, _) destructurings would be easier to follow.
  • let (start, key, false) = scan(text).value? else { return None }; is correct but easy to misread. A comment or an if listed { return None } would be clearer.

AGENTS.md

  • No new user-visible literals, so tl! doesn't apply. unwrap() appears only in the test, which is allowed. The docs were updated (docs/user-guide/analysis.md), and the test covers array, numeric, table and negative cases. Nothing was found against the guidelines. I didn't check cargo xtask ci or the layering.

Change size

Compared: the whole PR against main (merge base c281e2a to PR head 3fedaf5)

Metric Value
Files changed 2 (added 0, modified 2, removed 0, renamed 0)
Size before 188.6 KB
Size after 196.6 KB
Total size change +8.0 KB
File Before After Change
crates/ui-egui/src/scriptcomplete.rs 146.3 KB 154.0 KB +7.7 KB
docs/user-guide/analysis.md 42.3 KB 42.7 KB +324 B

@github-actions

github-actions Bot commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

📖 Docs preview

Item Value
Preview https://preview.rustgis.app/pr-856/
Web app https://preview.rustgis.app/pr-856/app/
Commit ee97b78

GitHub Pages takes a minute or two to publish a new commit. Newer commits from the fork aren't published until a maintainer adds the preview label (once per commit).

@github-actions

Copy link
Copy Markdown
Contributor

Code review

I read the whole diff (scriptcomplete.rs and the matching docs). I couldn't build or run anything here, so the tests and cargo xtask ci are unchecked. This PR extends script autocompletion so that keys the command description types as field, [fields] or [numeric fields] offer fields, including inside their arrays. It also adds TableFields for layer.geocode's "table". I found nothing that should block merging.

Bugs

  • None found. I traced these cases through scan, value_at, offer_at and tool_value:
    • The new listed flag only marks values directly inside a top-level key's array (depth 2).
    • value_of ignores listed values, so "layer": ["Cities"] no longer counts as the line's layer.
    • tool_value refuses arrays.
    • A listed word for a key that isn't a list-of-fields type returns None.
  • Low confidence, minor: entry_field_type uses starts_with("field"), so a future type word such as fieldset or fields_x would count as a field type. Matching the word exactly (up to a space, |, ] or ,) would be safer. Nothing in the current descriptions triggers it.
  • Low confidence, minor: value_of(line, "table")? makes the whole offer return None when "table" is given but isn't a plain string. The tests show this is intended (the "table": 7 case), though it also hides any fallback.

Security

  • No issues. The new code uses get()-style slicing (script.get(..)?, byte_at), contains no unwrap, expect or panic, and has no unsafe code.

Performance

  • No issues. offer_at now formats and rescans the line once per field offer to drop fields the array already lists. That is cheap for single-line script commands.

Quality

  • scriptcomplete.rs:117-118 (the value_of chain) is long. Confidence is low; cargo fmt in CI will say whether it passes.
  • The docs name "sort": " in report.new and "fields": [" in data.explore (numeric only). I couldn't confirm that those parameter descriptions use the field and [numeric fields] types the code depends on. The tests do cover data.explore, so the main risk is the report.new example (low confidence).
  • The Fields/TableFields branches duplicate logic in item_help and item_icon. That is fine at this size.

AGENTS.md

  • Everything runs through the command-description parser, with no new dialogs or UI-only commands.
  • There is no new user-visible text, so tl! doesn't apply.
  • Regression tests were added: keys_that_take_fields_offer_them_in_arrays_too and an_entrys_type_says_whether_it_takes_fields.
  • The docs are updated.
  • Please confirm cargo xtask ci passes before merging.

Change size

Compared: the whole PR against main (merge base c281e2a to PR head ee97b78).

Metric Value
Files changed 2 (added 0, modified 2, removed 0, renamed 0)
Size before 188.6 KB
Size after 198.0 KB
Total size change +9.3 KB
File Before After Change
crates/ui-egui/src/scriptcomplete.rs 146.3 KB 155.3 KB +9.0 KB
docs/user-guide/analysis.md 42.3 KB 42.7 KB +324 B

@jgregoriorincon
jgregoriorincon marked this pull request as ready for review October 24, 2026 23:54
@github-actions

Copy link
Copy Markdown
Contributor

Code review

This PR extends the script-completion offers in crates/ui-egui/src/scriptcomplete.rs. Keys whose description gives a field type now offer fields, inside their arrays too (#846). Numeric-only keys offer numeric fields, and layer.geocode offers its "table"'s fields. I read the whole diff and found nothing that needs a change. I had no Bash tool, so I could not compile or run the tests.

Bugs

  • None found.
    • The new scan tuple ((usize, String, bool)) is handled consistently in every consumer: value_at, value_of, tool_value and offer_at.
    • value_of ignores values that sit in an array. A line like {"layer": ["Cities"], "field": ""} therefore no longer takes "Cities" as the line's layer, and the test covers this.
    • In tool_value, the let (start, key, false) = … else { return None } refutable pattern is valid Rust. It correctly rejects arrays for tool parameters.
    • The layer.geocode path reads "table" through value_of, so a number or an escaped string returns None instead of falling back to the layer's fields. A table name that doesn't exist also returns None.

Security / Never-crash

  • No unwrap, expect, indexing or unsafe in the non-test code. Slicing uses get() and byte_at, so hostile script text can't panic.

Performance

  • Nothing notable. field_type re-parses the command's parameter description on every offer, and scan runs once more to collect the fields already given. Both are small, per-keystroke and bounded by line length.

Quality

  • Low confidence, nit: the value_of one-liner (about line 117) is now very long and probably wants a line break. cargo xtask ci checks fmt, so it should show up there.
  • Low confidence: entry_field_type guesses a list from any [ in the type text before (. A type such as field | [names] would be misread as a list of fields. The current descriptions seem fine, and the tests cover the known shapes. A comment noting the convention would help whoever writes the next parameter description.

AGENTS.md

  • No new user-visible literals, no new dependencies and no layering change. The docs in docs/user-guide/analysis.md are updated.
  • The regression tests include the hostile cases (deeper arrays, a missing table, a numeric table value).

Change size

Compared: the whole PR against main (merge base c281e2a to PR head ee97b78).

Metric Value
Files changed 2
Added 0
Modified 2
Removed 0
Renamed 0
Size before 188.6 KB
Size after 198.0 KB
Total size change +9.3 KB
File Before After Change
crates/ui-egui/src/scriptcomplete.rs 146.3 KB 155.3 KB +9.0 KB
docs/user-guide/analysis.md 42.3 KB 42.7 KB +324 B

@giswqs
giswqs merged commit 305784f into opengeos:main Oct 25, 2026
21 checks passed
@jgregoriorincon
jgregoriorincon deleted the feat/script-field-arrays branch October 25, 2026 03:37
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.

[Feature]: Offer field names in arrays and in keys that take a field in the Script pane

2 participants