Address substantive review comments on exact print proposal - #1
Closed
jappeace-sloth wants to merge 8 commits into
Closed
Address substantive review comments on exact print proposal#1jappeace-sloth wants to merge 8 commits into
jappeace-sloth wants to merge 8 commits into
Conversation
- Fix broken code example (now parseable Haskell) - Add second example: adding an exposed module (ulysses4ever request) - Define TWG acronym (Technical Working Group) on first use - Rewrite barbies section with actual code from gpd-barbie branch - Explain type machinery: HasAnnotation kind, type families, Trivia types - Address TypeError question: closed type family is exhaustive, no catch-all needed - Add Trees That Grow comparison and cabal-check compatibility discussion - Add comma/separator handling section with implementation details - Expand backwards compatibility section with concrete migration path - Add GPD vs Field discussion explaining why GPD was chosen - Identify potentially affected downstream packages (gtk2hs, gi-gtk) - Detail maintainer impact: what changes when adding new fields - Fix dangling half-sentences and improve document flow - Replace HTML list with markdown - Link to gpd-barbie implementation branch Prompt: go over the proposal review comments, list substantive unaddressed ones, then make a PR answering them inline with the proposal. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add sections addressing the deeper technical questions raised by Bodigrim, mpickering, and ulysses4ever in the cabal issue discussion: - Monoidal field merging: explain how PreserveGrouping distinguishes two separate build-depends fields from one merged field - Spaces inside version bounds: explain recursive parameterisation of VersionRangeWith storing SurroundingText at each AST node - Relationship to other lossless parsing approaches: compare with ruamel.yaml, rowan/Swift CST, GHC Exact Print Annotations - Eliminating the namespace/side-table: explain how the barbies approach avoids the fundamental lookup ambiguity problem Prompt: ulysses4ever refers to some of the final comments in haskell/cabal#11227, address that as well with the barbie approach. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Tested both packages against the gpd-barbie branch: - haskell-gi (CabalHooks.hs): compiles unchanged — its GPD field accesses (condLibrary, condTreeData, exposedModules, libBuildInfo, autogenModules) all reduce to original types under HasNoAnn. - gtk2hs (Gtk2HsSetup.hs): compiles unchanged — only uses resolved PackageDescription, never touches GPD directly. Also fix code example: condTreeConstraints was removed from CondTree on upstream master (not a barbies change). Updated the example to modify targetBuildDepends via condTreeData/libBuildInfo instead. Prompt: build gtk2hs/gi-gtk against gpd-barbie to test backwards compatibility. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The barbies prototype does not roundtrip yet. Modifying FieldGrammar to thread annotations is the hardest part; a first simple test is expected soon. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The ZuriHac prototype bypassed FieldGrammar entirely and could roundtrip basic files. The barbies approach integrates with FieldGrammar via dual HasNoAnn/HasAnn instances, which is better integrated but makes the field grammar work harder. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
jappeace
reviewed
Apr 15, 2026
jappeace
reviewed
Apr 15, 2026
- Remove AttachWith and PreserveGrouping from the code listing; only show AnnotateWith as the core idea, mention the others in prose. - Show the intended end state for GPD with all component fields parameterised (PackageDescriptionWith m, LibraryWith m, etc.), with a note that the current prototype only has condLibrary done. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Show the real shape: m propagates through LibraryWith -> BuildInfoWith, not at GPD level directly. Use ... for brevity. Show the chain from GPD to Library to BuildInfo where AnnotateWith actually appears. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The implementation will recursively parameterise PackageDescriptionWith m, PackageFlagWith m, and annotate gpdScannedVersion at the GPD level, not just propagate through condLibrary. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Comments addressed:
cabal-add/ HLS use case).gpd-barbiebranch, explaining the type machinery step by step.HasAnnotationconstructors, so noTypeErrorcatch-all is needed.SurroundingTexttrivia preserves leading/trailing comma styles.gtk2hsandgi-gtkas known GPD-touching packages; explained why most packages won't be affected.HasAnninstance, roundtrip tests catch missing annotations).<ul>/<li>with markdown list.Test plan
gpd-barbieimplementation🤖 Generated with Claude Code