Skip to content

Replace editor attribute classes with attribute map and validation - #14936

Merged
magnesj merged 11 commits into
devfrom
validation-replace-line-editor-validators-review
Oct 17, 2026
Merged

magnesj merged 11 commits into
devfrom
validation-replace-line-editor-validators-review

Conversation

@magnesj

@magnesj magnesj commented Oct 15, 2026

Copy link
Copy Markdown
Member

Summary

Replaces the per-editor PdmUi*EditorAttribute classes with a generic string-keyed attribute map on PdmUiItem (setAttribute / attribute / removeAttribute / attributeNames). Each editor declares its supported keys (Keys struct and SUPPORTED_ATTRIBUTES), and PdmUiItem::validateAttributes warns about unsupported keys. Line editor validators are replaced by attribute-based validation.

Review guide

Commits are grouped by layer, so review them in order:

  1. caf: attribute map and validation in cafPdmUiCore
  2. caf: removal of the editor attribute classes and per-editor keys in cafUserInterface
  3. caf: unit tests for attributes and editors
  4. caf: cafTestApplication updates (AUTORCC, windeployqt)
  5. ResInsight: migration of the Summary data model
  6. ResInsight: migration of the rest of the project data model
  7. ResInsight: migration of commands, UI and application code
  8. docs: agent guidelines

Intermediate commits may not build individually, because ApplicationLibCode depends on the removal of the caf attribute classes. Build and test the branch tip.

Testing

  • AppFwk standalone unit tests pass (cafPdmCore, cafPdmXml, cafProjectDataModel, cafPdmScripting, cafUserInterface).

@magnesj
magnesj requested a review from kriben October 15, 2026 07:10
@magnesj
magnesj marked this pull request as ready for review October 15, 2026 07:10
@magnesj magnesj self-assigned this Oct 15, 2026
Comment thread ApplicationLibCode/ProjectDataModel/Polygons/RimPolygonAppearance.cpp Outdated

@kriben kriben left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The seismic clip-value validator was removed without an equivalent replacement.

RimSeismicData and RimSeismicDifferenceData previously installed a QDoubleValidator(0.00001, infinity, 6) for m_userClipValue. Unlike the other migrated validators, neither a field range nor a map-based validator replaces it. The UI can therefore accept zero or negative clip values, after which updateDataRange() silently converts values <= 0 to the maximum data value.

Please restore equivalent validation, preferably centrally on the inherited pair-valued field while preserving its disabled 0.0 sentinel.

Verification performed:

  • git diff --check passed
  • cafUserInterface built successfully
  • Full ResInsight build succeeded

@magnesj
magnesj force-pushed the validation-replace-line-editor-validators-review branch from 335dbbd to 02c8d62 Compare October 16, 2026 07:50
@magnesj
magnesj force-pushed the validation-replace-line-editor-validators-review branch from 02c8d62 to 8b82c31 Compare October 16, 2026 07:59

@kriben kriben left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@magnesj
magnesj merged commit 3de9b3c into dev Oct 17, 2026
23 checks passed
@magnesj
magnesj deleted the validation-replace-line-editor-validators-review branch October 17, 2026 11:23
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.

2 participants