[Feature] _SettingsDialogState is 7,295 lines across 27 parts in one shared private scope #631

Closed
opened 2026-07-22 16:21:57 +00:00 by brenno · 3 comments
Owner

Found in the pre-publication architecture review. Relates to #518 (structural debt), which names TabsNotifier, DeckNotifier and app_shell but not this one.

Evidence: lib/widgets/dialogs/settings_dialog.dart#_SettingsDialogState appears in classSizeBaseline at 7,295 lines — 2.1× the next largest (_FullscreenPresenterState, 3,419) and 7.3× the ceiling. It is spread over 27 part files and 21 extension … on _SettingsDialogState, covering twenty unrelated domains: general, storage, presentation, display, colours, profile, WebDAV, git, S3, AI, privacy, security, docs, modules, checklists, standards, search, search index, local CVE database, about.

Why this matters now: the issue is not the size but the shared private scope. Each of those 27 files can reach every field of the other 26, so a change to the CVE panel syntactically touches the S3 credential fields. That is the most expensive kind of coupling to unpick later, because no gate sees it and the compiler gives no signal at all. Before publication this is one author; afterwards it is the file contributors will want to enter first — adding a setting is the classic first pull request — and the place they can move least safely.

Proposal: not all of it. Pull the four credential forms (parts/settings_dialog_webdav_form.dart, _git_form, _s3_form, _ai_form) out of the part scope into their own StatefulWidgets with an explicit callback API, then lower classSizeBaseline by what you removed. That is the seam with the sharpest boundary — credentials do not belong in a shared scope in any case — and the rest can stay.

Found in the pre-publication architecture review. Relates to #518 (structural debt), which names `TabsNotifier`, `DeckNotifier` and `app_shell` but not this one. **Evidence:** `lib/widgets/dialogs/settings_dialog.dart#_SettingsDialogState` appears in `classSizeBaseline` at **7,295** lines — 2.1× the next largest (`_FullscreenPresenterState`, 3,419) and 7.3× the ceiling. It is spread over **27 `part` files** and **21 `extension … on _SettingsDialogState`**, covering twenty unrelated domains: general, storage, presentation, display, colours, profile, WebDAV, git, S3, AI, privacy, security, docs, modules, checklists, standards, search, search index, local CVE database, about. **Why this matters now:** the issue is not the size but the **shared private scope**. Each of those 27 files can reach every field of the other 26, so a change to the CVE panel syntactically touches the S3 credential fields. That is the most expensive kind of coupling to unpick later, because no gate sees it and the compiler gives no signal at all. Before publication this is one author; afterwards it is the file contributors will want to enter first — adding a setting is the classic first pull request — and the place they can move least safely. **Proposal:** not all of it. Pull the four credential forms (`parts/settings_dialog_webdav_form.dart`, `_git_form`, `_s3_form`, `_ai_form`) out of the `part` scope into their own `StatefulWidget`s with an explicit callback API, then lower `classSizeBaseline` by what you removed. That is the seam with the sharpest boundary — credentials do not belong in a shared scope in any case — and the rest can stay.
Author
Owner

Verkend en niet gebouwd, met de kaart erbij zodat de volgende sessie niet opnieuw hoeft te zoeken.

De premisse is deels achterhaald. De vier *_form.dart-bestanden die je noemt zijn géén extensies op _SettingsDialogState meer — het zijn gewone klassen (WebdavForm, GitForm, S3Form, AiForm) die hun eigen controllers, sleutelhangerboekhouding en testuitslag dragen. De toestand van de vier inloggegevensformulieren staat dus al buiten de gedeelde scope, en die 381 regels tellen niet mee voor het klasseplafond.

Wat er wél nog in zit is de weergave: settings_dialog_webdav.dart (414), _git.dart (375), _s3.dart (292) en _ai.dart (314) zijn alle vier nog extension … on _SettingsDialogState. Samen zo'n 1.400 regels van de 7.100.

Waarom het niet in één keer los te trekken is. Die vier panelen leunen op vier gedeelde leden uit ándere parts:

  • _secretField (settings_dialog_secret.dart) — het wachtwoordveld met sleutelhangerlogica, gebruikt door alle vier;
  • _sectionTitle (settings_dialog_search.dart) — een opmaakhelper die overal in het dialoog zit;
  • _confirmCertificate (settings_dialog_storage.dart) — de certificaatbevestiging;
  • _rebuild — de setState van de state zelf.

De eerste twee moeten dus mee omhoog naar gedeelde widgets vóór de panelen kunnen verhuizen, en de derde wordt een callback. Dat is de echte volgorde van het werk, en het is meer dan een verplaatsing.

Waarom ik het nu niet doe: dit raakt de invoer van inloggegevens, het is de ene plek waar een stille gedragswijziging duur is, en het vraagt de aandacht die je aan het eind van een lange ronde niet meer hebt. Liever goed dan vandaag.

Claim staat er niet op, dus een volgende sessie kan hem zo oppakken.

Verkend en niet gebouwd, met de kaart erbij zodat de volgende sessie niet opnieuw hoeft te zoeken. **De premisse is deels achterhaald.** De vier `*_form.dart`-bestanden die je noemt zijn géén extensies op `_SettingsDialogState` meer — het zijn gewone klassen (`WebdavForm`, `GitForm`, `S3Form`, `AiForm`) die hun eigen controllers, sleutelhangerboekhouding en testuitslag dragen. De *toestand* van de vier inloggegevensformulieren staat dus al buiten de gedeelde scope, en die 381 regels tellen niet mee voor het klasseplafond. Wat er wél nog in zit is de **weergave**: `settings_dialog_webdav.dart` (414), `_git.dart` (375), `_s3.dart` (292) en `_ai.dart` (314) zijn alle vier nog `extension … on _SettingsDialogState`. Samen zo'n 1.400 regels van de 7.100. **Waarom het niet in één keer los te trekken is.** Die vier panelen leunen op vier gedeelde leden uit ándere parts: - `_secretField` (`settings_dialog_secret.dart`) — het wachtwoordveld met sleutelhangerlogica, gebruikt door alle vier; - `_sectionTitle` (`settings_dialog_search.dart`) — een opmaakhelper die overal in het dialoog zit; - `_confirmCertificate` (`settings_dialog_storage.dart`) — de certificaatbevestiging; - `_rebuild` — de `setState` van de state zelf. De eerste twee moeten dus mee omhoog naar gedeelde widgets vóór de panelen kunnen verhuizen, en de derde wordt een callback. Dat is de echte volgorde van het werk, en het is meer dan een verplaatsing. **Waarom ik het nu niet doe:** dit raakt de invoer van inloggegevens, het is de ene plek waar een stille gedragswijziging duur is, en het vraagt de aandacht die je aan het eind van een lange ronde niet meer hebt. Liever goed dan vandaag. Claim staat er niet op, dus een volgende sessie kan hem zo oppakken.
Author
Owner

Opgepakt. Tak: refactor/settings-panelen-uit-gedeelde-scope-631. Reikwijdte in deze ronde, in de volgorde die de vorige sessie in kaart bracht: eerst _sectionTitle en _secretField omhoog naar gedeelde widgets (lib/widgets/dialogs/settings/), daarna de S3- en WebDAV-panelen uit de part-scope naar eigen widgets met een expliciete callback-API, en classSizeBaseline omlaag met wat er weg is. Git en AI volgen in een tweede ronde als deze rond is.

Opgepakt. Tak: `refactor/settings-panelen-uit-gedeelde-scope-631`. Reikwijdte in deze ronde, in de volgorde die de vorige sessie in kaart bracht: eerst `_sectionTitle` en `_secretField` omhoog naar gedeelde widgets (`lib/widgets/dialogs/settings/`), daarna de S3- en WebDAV-panelen uit de `part`-scope naar eigen widgets met een expliciete callback-API, en `classSizeBaseline` omlaag met wat er weg is. Git en AI volgen in een tweede ronde als deze rond is.
Author
Owner

Gebouwd en op main: 78a84d48 (PR #716). Zeven commits, de ratchet per stap meegezakt.

De premisse achteraf. Je voorstel was "trek de vier inloggegevensformulieren uit de part-scope". De vorige sessie stelde vast dat de vier *_form.dart-bestanden die je noemde al gewone klassen waren en dat het echte werk in de vier panelen zat, met vier gedeelde leden als blokkade. Die volgorde klopte, en is aangehouden.

Wat er staat. lib/widgets/dialogs/settings/, buiten de gedeelde scope:

  • WebdavPanel, S3Panel en GitPanel — gewone widgets met een expliciete API: het formulier dat ze bewerken, een ConfirmCertificate-callback, en een onChanged. Dat laatste is geen decoratie: de statusregel achter de verbindingsnaam toont de uitslag van de verbindingstest en staat buiten het paneel.
  • De vier formulierklassen (WebdavForm, S3Form, GitForm, AiForm) en KeychainSecret.
  • De drie gedeelde leden die je als blokkade zou aanwijzen: _sectionTitle werd SettingsSectionTitle, dat zijn anker en het oplichten uit een SettingsSectionAnchors-InheritedWidget haalt in plaats van uit de venstertoestand — dát is wat een paneel buiten de scope laat leven. _webdavField (dat de naam van één bron droeg maar door alle vier gebruikt werd) en _secretField werden SettingsTextField en SettingsSecretField. _confirmCertificate werd een callback.

_SettingsDialogState gaat van 7.295 naar 6.043 regels.

Waarom het meer is dan verplaatsen. Elk paneel is nu te tekenen en te toetsen zonder de dialoog te openen; die tests staan erbij. Ze bewaken het geval dat in alle drie hetzelfde stil misgaat: een instelling wijzigen die de vorige verbindingstest ongeldig maakt zonder de groene vink weg te halen. Dan meldt het paneel "verbinding gelukt" over een verbinding die het niet geprobeerd heeft.

Wat de verhuizing aan het licht bracht, en het is het interessantste van deze ronde. Twaalf zichtbare hints — voorbeeld-URL's, een regio, een branchnaam, een submap — liepen buiten l10n.d() om. Ze waren niet nieuw: _webdavField was een extension-methode, en daar kijkt de datastroomanalyse van check_hardcoded_text niet doorheen. De poort was dus blind voor precies dat stuk code, jarenlang, en werd ziend op het moment dat het een gewone widget werd. Tien zijn identifiers (main, decks, eu-central-1) en staan nu in unchangedInAllLanguages; twee waren echt Nederlands en zijn in 31 talen vertaald; en de twee Nederlandse voorbeelddomeinen (cloud.voorbeeld.nl) zijn example.com geworden — een voorbeeldhostnaam die in elke taal hetzelfde is, is beter dan eenendertig varianten van een adres dat niet bestaat.

Eén ding dat nog blind is, en dat laat ik hier staan omdat het geen bijvangst hoort te zijn: dezelfde blindheid geldt nog voor élke andere extension-methode in parts/ die tekst doorgeeft. Deze ronde ruimde de vier panelen op; de poort ziet de rest nog steeds niet.

Wat er níét in zit. Het AI-tabblad. Dat is geen paneel in de verbindingenlijst maar een tabblad dat zijn velden bij het openen initialiseert en bij Opslaan wegschrijft — een andere ingreep dan deze drie. AiForm is er wel uit, dus de klasse die de API-sleutel vasthoudt zit niet meer in een scope waar zesentwintig onderwerpen bij kunnen.

Ik sluit dit issue: je voorstel — de vier inloggegevensformulieren uit de part-scope, met de baseline omlaag — is af. Het AI-tabblad is een eigen ronde en geen restpost van deze; komt die eraan, dan is een nieuw issue eerlijker dan dit er open voor houden.

Poort: make check groen (5.858 tests), make test-golden groen (33), make sbom-verify schoon, herbaseerd op main ná #715 en daar opnieuw gedraaid.

Gebouwd en op main: `78a84d48` (PR #716). Zeven commits, de ratchet per stap meegezakt. **De premisse achteraf.** Je voorstel was "trek de vier inloggegevensformulieren uit de `part`-scope". De vorige sessie stelde vast dat de vier `*_form.dart`-bestanden die je noemde al gewone klassen waren en dat het echte werk in de vier *panelen* zat, met vier gedeelde leden als blokkade. Die volgorde klopte, en is aangehouden. **Wat er staat.** `lib/widgets/dialogs/settings/`, buiten de gedeelde scope: - `WebdavPanel`, `S3Panel` en `GitPanel` — gewone widgets met een expliciete API: het formulier dat ze bewerken, een `ConfirmCertificate`-callback, en een `onChanged`. Dat laatste is geen decoratie: de statusregel achter de verbindingsnaam toont de uitslag van de verbindingstest en staat buiten het paneel. - De vier formulierklassen (`WebdavForm`, `S3Form`, `GitForm`, `AiForm`) en `KeychainSecret`. - De drie gedeelde leden die je als blokkade zou aanwijzen: `_sectionTitle` werd `SettingsSectionTitle`, dat zijn anker en het oplichten uit een `SettingsSectionAnchors`-InheritedWidget haalt in plaats van uit de venstertoestand — dát is wat een paneel buiten de scope laat leven. `_webdavField` (dat de naam van één bron droeg maar door alle vier gebruikt werd) en `_secretField` werden `SettingsTextField` en `SettingsSecretField`. `_confirmCertificate` werd een callback. `_SettingsDialogState` gaat van **7.295 naar 6.043 regels**. **Waarom het meer is dan verplaatsen.** Elk paneel is nu te tekenen en te toetsen zonder de dialoog te openen; die tests staan erbij. Ze bewaken het geval dat in alle drie hetzelfde stil misgaat: een instelling wijzigen die de vorige verbindingstest ongeldig maakt zonder de groene vink weg te halen. Dan meldt het paneel "verbinding gelukt" over een verbinding die het niet geprobeerd heeft. **Wat de verhuizing aan het licht bracht, en het is het interessantste van deze ronde.** Twaalf zichtbare hints — voorbeeld-URL's, een regio, een branchnaam, een submap — liepen buiten `l10n.d()` om. Ze waren niet nieuw: `_webdavField` was een `extension`-methode, en daar kijkt de datastroomanalyse van `check_hardcoded_text` niet doorheen. De poort was dus blind voor precies dat stuk code, jarenlang, en werd ziend op het moment dat het een gewone widget werd. Tien zijn identifiers (`main`, `decks`, `eu-central-1`) en staan nu in `unchangedInAllLanguages`; twee waren echt Nederlands en zijn in 31 talen vertaald; en de twee Nederlandse voorbeelddomeinen (`cloud.voorbeeld.nl`) zijn `example.com` geworden — een voorbeeldhostnaam die in elke taal hetzelfde is, is beter dan eenendertig varianten van een adres dat niet bestaat. **Eén ding dat nog blind is, en dat laat ik hier staan omdat het geen bijvangst hoort te zijn:** dezelfde blindheid geldt nog voor élke andere `extension`-methode in `parts/` die tekst doorgeeft. Deze ronde ruimde de vier panelen op; de poort ziet de rest nog steeds niet. **Wat er níét in zit.** Het AI-*tabblad*. Dat is geen paneel in de verbindingenlijst maar een tabblad dat zijn velden bij het openen initialiseert en bij Opslaan wegschrijft — een andere ingreep dan deze drie. `AiForm` is er wel uit, dus de klasse die de API-sleutel vasthoudt zit niet meer in een scope waar zesentwintig onderwerpen bij kunnen. Ik sluit dit issue: je voorstel — de vier inloggegevensformulieren uit de `part`-scope, met de baseline omlaag — is af. Het AI-tabblad is een eigen ronde en geen restpost van deze; komt die eraan, dan is een nieuw issue eerlijker dan dit er open voor houden. Poort: `make check` groen (5.858 tests), `make test-golden` groen (33), `make sbom-verify` schoon, herbaseerd op main ná #715 en daar opnieuw gedraaid.
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
LibreKAT/Ocideck#631
No description provided.