Add exact print proposal - #5
Conversation
|
@jappeace could you explain what's the relation to the earlier HF proposal? |
The start of the document says "Note that this is a copy of the TWG proposal: haskellfoundation/tech-proposals#6", but indeed I'm curious how literally this is a copy, in particular, if the experience gained when implementing and discussing lead to some modifications or maybe if this is the baseline over which the modifications are expected to be made. |
|
There's probably no harm in copying old stuff, but I personally hoped that we can make some profit out of our process and discuss some details that poped up during the past couple months in a new document. If creating a whole new document sounds like too much, a new section in the copy would be fine as well... |
|
I put the somewhat relevant parts of TWG proposal into the relevant sections of the cabal proposal template. unlike the original proposal says, the common stanzas have been investigated here: and unlike the original proposal says, comment parsing is now pretty solid with this PR: I think these are the only two major implementation issues left from the original TWG proposal: Both of these are investigated by a prototype focussing on "trivia", because FieldGrammar handles both conditionals as well as the comma's. |
|
Hello, we discussed this proposal tonight ad the cabal dev-meeting. We are generally happy with it, but there are things we would like to get a better idea of.
Thanks for having written this document! |
|
I'd like to note that the current POC is a step in the direction of answering some of these questions (I did mention this in the meeting), in particular testing it against |
|
Right, the POC is a great contribution. Maybe it could serve to update the proposal a bit, especially the parts the maintainers are keen about, as listed by Francesco? This proposal, as opposed to the TWG one, has the benefit of being submitted after some considerable prototyping and some feedback, so it can stand on much less hypothetical foundations than the TWG one. It should now be easier to involve many cabal "stakeholders" in the discussion, soliciting feedback about, e.g., backward compatibility and potential migration policy. In any case, thank you very much for all this recent work, we are thrilled to see such a professional approach and we hope this can be brought to fruition with minimal friction. |
|
@ffaf1 Please help me understand what defines a major user, Cabal has around 250 direct depenencies on hackage. The other points I should be able to address with current information. |
|
@jappeace Apologies for the late reply, I am not ignoring you, I am just trying to check which project using It would of course be silly and wrong to ask you to collect feedback from each I recall in the last cabal-dev meet @geekosaur had an example of such a “probably impacted” dependency, but I forgot which one it was. |
|
I think I mentioned the custom |
|
I cleaned up the document, please review whenever you've time 🙂 |
|
Oh, great! Yes, we will. |
|
People expressed the need for a few more days to review this proposal. I'm sure we will decide on it in the meeting in 2 weeks. @jappeace: please let us know whenever there's any deadline either regarding this proposal or the implementation PRs themselves --- almost all the work on cabal is done by volunteers, so we can't commit to any review timelines, but we do try our best, especially if notified in advance. Thank you! |
|
@Mikolaj I understand this is mostly driven by volunteers yes, thanks for your time :) |
|
@jappeace asked to postpone approving the proposal, so the next deadline it the cabal chat in 2 weeks. |
|
Would it be possible to showcase the approach that you take by providing examples of how some of the common exact-print-related tasks could be solved after you're done? For instance, how to implement (some of) |
|
Regarding the procedural side, since there are many outstanding comments right now, we have decided once more to postpone the decision about this proposal by two weeks. Apologies! |
| We developed an alternative approach in the barbies section. | ||
|
|
||
| ### (TWG) Namespace approach | ||
| where `hackagePackage` is a cabal package found on Hackage. |
| In this approach we try to eat the cake and have it too with some clever type machinery. | ||
| We want to be able to annotate `GenericPackageDescription` with exact data, | ||
| however we also want to keep it backwards compatible. | ||
| Here we we're inspired by the [barbies](https://hackage.haskell.org/package/barbies) |
There was a problem hiding this comment.
| Here we we're inspired by the [barbies](https://hackage.haskell.org/package/barbies) | |
| Here we are inspired by the [barbies](https://hackage.haskell.org/package/barbies) |
| This will allow us to retain the shape of the parse result. | ||
| Solving the following problems: | ||
|
|
||
| In field grammar we sidestep the problem where we fail to lookup trivia. |
There was a problem hiding this comment.
What problem is it, can you elaborate? The whole paragraph is impossible to follow for me. I'd need to get more elaborated discussion.
There was a problem hiding this comment.
It's just hard to debug because you don't know if:
- The key is actually missing
- the path got mangled somehow.
- One of the above typeclasses in field grammar didn't pass the argument
- If this is the case you need to track down which. To do this you need to write golden tests and dum p the AST.
It's just time consuming, and it appears kidn of fragile.
This barbies approach is compiler driven, which is a lot of work but it's a lot less error prone
|
In one of the earlier attempts at it from you Bodigrim asked a bunch of questions about handling particular syntax. E. g. a comma in a list of packages inside build-depends (for instance) can go on either line. Will you be able to capture this sort of differences with your approach? Generally, there were a bunch of good questions in the PRs you submitted earlier. It'd be wise to go over then and address them in the proposal. E. g. one of the big points was whether you have to touch GPD or would it be possible to stick with Field. This is only mentioned in passing in the proposal, while I'd expect a more in-depth discussion of this decision. |
| All the difficulty lies in figuring out where to place a dependency; | ||
| we made a decision here to do it in the main library assuming it exists. | ||
| We also assumed there would be no conditionals. |
There was a problem hiding this comment.
Anecdotally, one of the very first feature requests for cabal-add was "Can I add a dependency to a common stanza?" How would your approach handle it? Common stanzas are not even present in GPD.
There was a problem hiding this comment.
I suppose you'd use the field gpdCommonStanza, see updated proposal
| let lib = condTreeData tree | ||
| in tree { condTreeData = lib { exposedModules = modName : exposedModules lib } } | ||
| ``` | ||
| This is the kind of operation that `cabal-add` or HLS could use. |
There was a problem hiding this comment.
This is the kind of operation that cabal-add already supports and HLS uses (through cabal-add).
| to modify it's own files? | ||
| It's hard to guarantee stability across projects if many functionalities | ||
| are distributed across many projects. | ||
| For example a newly introduced tool called [cabal-add](https://github.com/Bodigrim/cabal-add), would need to take into account any |
There was a problem hiding this comment.
As of 2026, it's no longer "a newly introduced" ;)
| The byte-for-byte roundtrip property holds where `hackagePackage` is a cabal package found on Hackage. | ||
|
|
||
| We explored two approaches. | ||
| The namespace (trivia-tree) approach works but progress was slow because: |
There was a problem hiding this comment.
A forward reference to a section, explaining the term would be helpful. At this point a reader has no idea what it is about.
| type GenericPackageDescription = GenericPackageDescriptionWith HasNoAnn | ||
| type GenericPackageDescriptionAnn = GenericPackageDescriptionWith HasAnn | ||
|
|
||
| data GenericPackageDescriptionWith (m :: HasAnnotation) = GenericPackageDescription |
There was a problem hiding this comment.
By the time we arrive to GPD, common stanzas have been already inlined and (I think) normalised. So any positions and annotations can refer to places far away from. Say, a list of library dependencies will contain items from any stanza or from the library section itself. How would you reconstruct the original Cabal file? How would you insert a new dependency in such list?
There was a problem hiding this comment.
It's deffered, now explained more elaborately in the proposal but we've a PR for that open too (although leana wants to work more on it before opening it to review).
| This keeps the machinery simple and avoids the pattern-match exhaustiveness problems | ||
| that Trees That Grow can introduce in GHC's own codebase. |
There was a problem hiding this comment.
I don't follow: the whole point of TTG is to avoid the issue with non-exhaustive pattern-matching?..
There was a problem hiding this comment.
Yes this was misdescribed, we just have a relatively simple HasAnn | HasNoAnn, TTG is more complex but addresses that as well in their own way.
| Our closed type family with two constructors avoids this. | ||
|
|
||
| + **cabal-fields / Field-level manipulation** (raised by @mpickering referencing cabal-add): | ||
| working at the `Field` level avoids touching GPD but loses type safety. |
There was a problem hiding this comment.
I don't really follow, what type safety are we talking about?
There was a problem hiding this comment.
you no longer have to work with raw strings, the text reflects that.
| A first simple roundtrip test based on the barbies approach is expected soon. | ||
|
|
||
| ### Exact printing | ||
| The `pretty` library doesn't have a newline primitive, and I find it hard to position elements exactly. |
There was a problem hiding this comment.
Hmmm? Why did we suddenly jump to pretty library? What does it have to do with the rest of the proposal?
There was a problem hiding this comment.
we found out that pretty isn't going to work in the trivia tree approach. This is why we introduced exact pretty which seems to make things a lot easier.
| but code that is polymorphic over the GPD parameter or that | ||
| pattern-matches on `GenericPackageDescriptionWith` directly will see `LibraryWith m`. | ||
|
|
||
| Additionally, common stanza merging will be postponed to allow |
There was a problem hiding this comment.
Postponing merging of common stanzas is a significant design change, which I have not seen discussed above or agreed upon.
There was a problem hiding this comment.
There is an entire seciton on this now
| e.g. `cabal init` could be described via `GenericPackageDescription`. | ||
|
|
||
| The HLS project would benefit from this effort; | ||
| it could add a dependencies plugin for example: https://github.com/haskell/haskell-language-server/issues/155 |
There was a problem hiding this comment.
The referenced issue is closed as completed.
| ([#4617](https://github.com/haskell/haskell-language-server/pull/4617)). | ||
|
|
||
| However, several more fine-grained operations remain open | ||
| that are difficult to implement at the `Field` level: |
There was a problem hiding this comment.
This phrasing (perhaps inadverently) seems to suggest that the operations listed below are not available through cabal-add. That's not so: both renaming and adding exposed-modules / other-modules are supported in cabal-add, even if HLS plumbing remains unfinished.
There was a problem hiding this comment.
how are you even supposed to know that? neither the readme nor the help command tells you what options are available 😅
There was a problem hiding this comment.
cabal-add-the-library, not cabal-add-the-executable. See https://hackage-content.haskell.org/package/cabal-add-0.2/docs/Distribution-Client-Add.html#t:AddConfig and https://hackage-content.haskell.org/package/cabal-add-0.2/docs/Distribution-Client-Rename.html#t:RenameConfig
| + `Nest` distributes over `Concat` and `StickyConcat` | ||
| but does **not** override `Place`: | ||
|
|
||
| ```hs |
very fancy
This reverts commit 81a9472.
|
We've accepted the proposal at today's regular cabal developer's meeting. This should not stop the discussion either here or in the PRs that will implement the proposal, because the design will naturally evolve all the time. Thank you everybody! |
|
Since the proposal was accepted, should we merge, perhaps, @Mikolaj? |
|
Huh, that's true, I missed that. But now we have a new, even better proposal for the same feature, so maybe it's less noise if we don't keep both merged? @jappeace: how do you prefer to settle this? |
|
better we focus on that other one #7 |
No description provided.