Skip to content

Read only config#82

Merged
graeme merged 2 commits into
mainfrom
read-only-config
Jul 5, 2026
Merged

Read only config#82
graeme merged 2 commits into
mainfrom
read-only-config

Conversation

@graeme

@graeme graeme commented Jul 5, 2026

Copy link
Copy Markdown
Collaborator

PR: Make Config feature read-only; remove env file editing

Summary

Removes all brew.env read/write infrastructure from the Config feature, making the Configuration page a read-only view of brew config output. The env-file editor added complexity and surface area that wasn't justified by the current UX; stripping it out brings the feature back to a simple, correct display of the config snapshot.

Changes

  • Deleted BrewEnvFile model and all CLI-layer env file codeBrewEnvFileLocator, BrewEnvFileParser, BrewEnvFileWriter and the BrewEnvFile domain model are gone.
  • Removed EnvFileRepository protocol and implementationBrewEnvFileRepository and the EnvFileRepository protocol are deleted, along with their registration in RepositoryEnvironment and the unimplemented stub.
  • Slimmed ConfigViewModel from ~400 lines to ~50 — draft state, dirty tracking, save/revert logic, dangerous-key confirmation flow, and pageState combining both repositories are all removed. The VM now exposes a single pageState: LoadState<BrewConfigSnapshot, String> backed only by ConfigRepository.
  • Deleted editor-only ViewModelsCaskOpts, ConfigPagePayload, EnvKeyDescriptor, EnvRowItem are removed entirely.
  • Removed ConfigEnvironmentEditorCard — the 365-line editor view is gone; ConfigView and ConfigColumnsRoot updated accordingly.
  • Added BrewConfigSnapshot+Placeholdable — small extension to support redacted placeholder rendering on the now-simplified config view.
  • Deleted all associated testsBrewEnvFileBrewSemanticsTests, BrewEnvFileLocatorTests, BrewEnvFileParserTests, BrewEnvFileWriterTests, BrewEnvFileRepositoryTests, CaskOptsTests, ConfigViewModelAdvancedTests, and ConfigViewModelEditingTests are removed. ConfigViewModelTests is trimmed to match the reduced VM surface.

Net: −3,333 lines, +49.

Why this split

The env-file writer was built speculatively. Keeping it required maintaining a parallel load/sync path alongside the config snapshot, handling merge conflicts between external file changes and in-progress edits, and classifying keys as dangerous or read-only — all for a feature that doesn't ship yet. Removing it now leaves a clean foundation; editing can be re-introduced as a deliberate, scoped piece of work if needed.

Testing

  • swift test --filter ConfigViewModelTests — all remaining config VM tests pass.
  • App builds and the Configuration tab loads and displays the brew config snapshot correctly.
  • No references to deleted types remain in the compiled target.

PR checklist

  • Have you followed this repository's contribution and workflow guidance?
  • Have you explained what changed and why this should land now?
  • Have you run relevant local checks for the changed scope?
  • Are changes scoped and free of unrelated modifications?

  • AI was used to generate or assist with generating this PR.
  • If yes, describe exactly how AI was used and what manual verification was performed.
    Claude Code removed most of the code and drafted this description from git diff main...HEAD. The diff, commit messages, and deleted files were reviewed manually to confirm accuracy before filing.

graeme and others added 2 commits July 5, 2026 19:46
Config tab now shows brew config output only — no editing.
Removed everything related to reading or writing brew.env:
BrewEnvFile model, BrewEnvFileParser, BrewEnvFileLocator,
BrewEnvFileRepository, EnvFileRepository protocol, and all
editor UI (ConfigEnvironmentEditorCard, EnvKeyDescriptor,
EnvRowItem, CaskOpts, ConfigPagePayload). ConfigViewModel
and ConfigView simplified to a single repository dependency.
All associated tests deleted; ConfigViewModelTests updated.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 5, 2026 09:56
@graeme graeme requested a review from MikeMcQuaid July 5, 2026 09:57

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR simplifies the Config feature by removing all brew.env read/write/editing infrastructure, leaving the Configuration page as a read-only view backed solely by the brew config snapshot.

Changes:

  • Removed the brew.env domain model, parser/writer/locator, repository interface + implementation, and all related tests.
  • Simplified ConfigViewModel and views to load/render only ConfigRepository state (read-only; copy/refresh only).
  • Added BrewConfigSnapshot: Placeholdable to support AsyncContentView’s redacted placeholder rendering after the VM simplification.

Reviewed changes

Copilot reviewed 30 out of 30 changed files in this pull request and generated no comments.

Show a summary per file
File Description
Tests/BrewRepositoriesTests/BrewEnvFileRepositoryTests.swift Deleted env-file repository tests (env editing removed).
Tests/BrewFeatureConfigTests/ConfigViewModelTests.swift Updated tests for the simplified ConfigViewModel init and behavior.
Tests/BrewFeatureConfigTests/ConfigViewModelEditingTests.swift Deleted editor-flow tests (editing removed).
Tests/BrewFeatureConfigTests/ConfigViewModelAdvancedTests.swift Deleted advanced editor behavior tests (editing removed).
Tests/BrewFeatureConfigTests/CaskOptsTests.swift Deleted HOMEBREW_CASK_OPTS advisory tests (editor removed).
Tests/BrewCLITests/BrewEnvFileWriterTests.swift Deleted env-file writer tests (writer removed).
Tests/BrewCLITests/BrewEnvFileParserTests.swift Deleted env-file parser tests (parser removed).
Tests/BrewCLITests/BrewEnvFileLocatorTests.swift Deleted env-file locator tests (locator removed).
Tests/BrewCLITests/BrewEnvFileBrewSemanticsTests.swift Deleted brew.env semantics tests (feature removed).
Sources/BrewRepositoryInterfaces/Protocols/EnvFileRepository.swift Deleted EnvFileRepository protocol.
Sources/BrewRepositoryInterfaces/PreviewSupport/PreviewSupport.swift Removed env-file preview repository wiring.
Sources/BrewRepositoryInterfaces/Fakes/Stubs.swift Removed StubEnvFileRepository fake.
Sources/BrewRepositories/BrewEnvFileRepository.swift Deleted live brew.env repository implementation.
Sources/BrewFeatureConfig/Views/ConfigView.swift Removed save/discard/editor UI; now copy/refresh + read-only cards.
Sources/BrewFeatureConfig/Views/ConfigEnvironmentEditorCard.swift Deleted the environment editor card UI.
Sources/BrewFeatureConfig/Views/ConfigColumnsRoot.swift Removed env-file repository dependency injection into ConfigView.
Sources/BrewFeatureConfig/ViewModels/EnvRowItem.swift Deleted editor row model types.
Sources/BrewFeatureConfig/ViewModels/EnvKeyDescriptor.swift Deleted env key catalogue/descriptor/editor-only types.
Sources/BrewFeatureConfig/ViewModels/ConfigViewModel.swift Reduced VM to config snapshot load/refresh + error mapping only.
Sources/BrewFeatureConfig/ViewModels/ConfigPagePayload.swift Deleted combined snapshot+env payload model.
Sources/BrewFeatureConfig/ViewModels/CaskOpts.swift Deleted cask opts advisory logic (editor removed).
Sources/BrewCore/Models/BrewEnvFile.swift Deleted BrewEnvFile domain model.
Sources/BrewCore/Models/BrewConfigSnapshot+Placeholdable.swift Added placeholder snapshot for AsyncContentView loading skeletons.
Sources/BrewCLI/Config/BrewEnvFileWriter.swift Deleted brew.env writer.
Sources/BrewCLI/Config/BrewEnvFileParser.swift Deleted brew.env parser.
Sources/BrewCLI/Config/BrewEnvFileLocator.swift Deleted brew.env locator.
Sources/BrewAppEnvironment/UnimplementedRepositories.swift Removed unimplemented env-file repository stub.
Sources/BrewAppEnvironment/RepositoryEnvironment.swift Removed EnvironmentValues.envFileRepository entry.
Homebrew/Features/MainWindow/Views/MainWindowView.swift Removed env-file repository preview injection.
Homebrew/BrewApp.swift Removed env-file repository from app composition + scene-phase invalidation.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@graeme graeme merged commit abf795a into main Jul 5, 2026
12 checks passed
@graeme graeme deleted the read-only-config branch July 5, 2026 11:43
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.

3 participants