Skip to content

Stabilize module settings grouping#91

Merged
kissetfall merged 2 commits into
ergohaven:mainfrom
IgorArkhipov:igor/module-settings-grouping-fix
Jul 19, 2026
Merged

Stabilize module settings grouping#91
kissetfall merged 2 commits into
ergohaven:mainfrom
IgorArkhipov:igor/module-settings-grouping-fix

Conversation

@IgorArkhipov

@IgorArkhipov IgorArkhipov commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

EN

Problem

Module settings classification relied on broad title substrings and created one group per firmware tab. Names such as Brightness Modules could be mistaken for right-controller settings, while a tab containing both left and right controller fields could not preserve side ownership reliably.

Fix

  • Parse normalized whole-word tokens instead of loose left, right, and module substrings.
  • Prefer explicit side, module_side, or moduleSide metadata when firmware provides it.
  • Split mixed controller tabs into stable Left, Right, Auto Layer, and Other groups.
  • Keep original firmware field titles and add side labels to generated mixed-tab group titles.
  • Admit generic controller tabs only when field metadata or recognized module field vocabulary proves they contain module settings.
  • Add fixtures for known split tabs, mixed tabs, explicit side metadata, auto-layer settings, and substring collisions.

This PR complements the diagnostics work in #88 without duplicating it.

Verification

  • cargo test --locked --all-targets: 195 passed.
  • cargo test module_settings_groups: 5 passed.
  • cargo clippy --locked --all-targets: passed with existing repository warnings.
  • python3 scripts/check_i18n.py: passed.
  • Scoped rustfmt --check and git diff --check: passed.
  • TARGET=aarch64-apple-darwin scripts/build_macos_app.sh: arm64 app, DMG, and ZIP built; plist and code-signature validation passed.
  • Built executable architecture validated as arm64 by the packaging script.

RU

Проблема

Группировка настроек модулей опиралась на широкие совпадения подстрок в заголовках и создавала одну группу на вкладку прошивки. Например, Brightness Modules могла ошибочно определяться как настройки правого контроллера, а вкладка с полями левого и правого контроллеров не сохраняла принадлежность сторон надежно.

Исправление

  • Вместо широкого поиска подстрок left, right и module используются нормализованные отдельные слова.
  • При наличии в прошивке приоритет отдается явным метаданным side, module_side или moduleSide.
  • Смешанные вкладки контроллеров разделяются на стабильные группы Left, Right, Auto Layer и Other.
  • Исходные названия полей прошивки сохраняются; созданные из смешанных вкладок группы получают обозначение стороны.
  • Общие вкладки контроллеров включаются только при наличии метаданных поля или распознанной лексики настроек модулей.
  • Добавлены фикстуры для отдельных вкладок сторон, смешанных вкладок, явных метаданных стороны, Auto Layer и конфликтующих подстрок.

PR дополняет диагностическую работу в #88, не дублируя ее.

Проверка

  • cargo test --locked --all-targets: прошли 195 тестов.
  • cargo test module_settings_groups: прошли 5 тестов.
  • cargo clippy --locked --all-targets: прошел с существующими предупреждениями репозитория.
  • python3 scripts/check_i18n.py: прошел.
  • Scoped rustfmt --check и git diff --check: прошли.
  • TARGET=aarch64-apple-darwin scripts/build_macos_app.sh: собраны arm64-приложение, DMG и ZIP; plist и подпись прошли проверку.
  • Архитектура исполняемого файла проверена скриптом упаковки как arm64.

@IgorArkhipov
IgorArkhipov force-pushed the igor/module-settings-grouping-fix branch from ea91e43 to a772434 Compare July 17, 2026 21:05
@IgorArkhipov
IgorArkhipov force-pushed the igor/module-settings-grouping-fix branch from a772434 to 9a9a462 Compare July 18, 2026 13:33

@kissetfall kissetfall left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The word-based classification works for the current rmk-eh definitions, but two issues need to be corrected before merge.

  1. Coalesce side groups across tabs. The new classifier can admit an existing Left Modules tab and a generic controller tab containing a Left Mode field, producing two Left groups. The current side selector renders both with the same localized Left half label and displays only one group at a time, making settings indistinguishable and effectively hiding part of that side. The parser should guarantee at most one Left and one Right group, or the state/UI must explicitly represent multiple named groups per side. Please add a regression test with multiple tabs contributing fields to the same side and verify every field remains accessible.
  2. Reuse the existing settings_title_words helper instead of adding the identical module_setting_words implementation in the same owning module.

Please retain regression coverage for the current K04 Left, Right, and Auto Layer definitions.

@IgorArkhipov

Copy link
Copy Markdown
Contributor Author

Addressed in 837d684.

  1. Parser now coalesces Left, Right, and Auto Layer groups across tabs, preserving field order and ensuring each side has one selector target.
  2. Replaced duplicate module_setting_words with existing settings_title_words.
  3. Added regression covering dedicated Left/Right/Auto Layer tabs plus generic controller fields. It verifies one group per kind, both selectable side indices, and retention of every QSID.
  4. Existing K04 Left, Right, and Auto Layer coverage remains in place.

@kissetfall kissetfall left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Re-reviewed head 837d684 against current main. The requested coalescing and helper reuse are implemented in the owning parser, all side fields remain reachable, and the regression covers multi-tab Left, Right, and Auto Layer grouping. Local validation passed: 226/226 tests, Clippy, i18n, rustfmt, diff-check, and release build. CI is 4/4 green.

@kissetfall
kissetfall merged commit 1fdda72 into ergohaven:main Jul 19, 2026
4 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.

2 participants