Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Exact printer #9436

Draft
wants to merge 41 commits into
base: master
Choose a base branch
from
Draft

Exact printer #9436

wants to merge 41 commits into from

Conversation

jappeace
Copy link
Collaborator

@jappeace jappeace commented Nov 12, 2023

My friend @tchoutri recommended me to not "just drop an orphan" on the door and get other maintainers involved early.
However I felt I already had disccused some approach, it didn't receive no comments unfortunatly,
So this is a PR with some early progress because it does render
the top level field names correctly, still many things have to be done however. (all help is welcome).

it's essentially follows the approach I outlined earlier
I added meta data to the GenericPackageDescription from the parser, fixed the compile errors (which weren't even all that many), added the function outlined and a test for that function with some random sample cabal file I found.

I feel this approach is a bit more feasable then the annotationless exact-printer, since it skips the "create a parser step", which reduces the amount of work involved and this approach has pretty decent backwards compatibility (since no parser is replaced).

Please leave your thoughts if you have any :)

TODO:

  1. Section rendering
  2. multiple section rendering.
  3. Add other tests for other cabal file examples.
  4. add comment support. I at least figured out where this is deleted.
  5. common stanza's get deleted.
    this is a rather hairy problem because the parser merges them.
    we need to store the existense of common stanza's in the meta file, then we need to record what merges happen, so we can undo them when printing. Because common stanzas aren't part of generic package we don't need to bother with making it witstand change.
  6. elif breaks
  7. Figure out build-depends, something like (eg Map (SectionName, PackageVersionConstraint) Originall)
  8. add round trip property so that the output should have a byte-for-byte round trip on
    • no-op parse/print and
    • print/parse tests.
  9. do this on all of hackage

@jappeace jappeace marked this pull request as draft November 12, 2023 22:41
@BurningWitness
Copy link

BurningWitness commented Nov 13, 2023

this approach is a bit more feasible then the annotationless exact-printer

I'm afraid without proper maintainer guidance the shortest answer will always look like the most feasible one.

My position on the parsing issue is admittedly atrociously idealistic: I think Cabal's current parsing approach is both incredibly unwieldy and not powerful enough (see Distribution.PackageDescription.FieldGrammar), and my solution is separating it into the manifest format, separate field formats, parsers and printers. The manifest format is then common for every .cabal file regardless of the version and even cabal.project; field formats don't have to be infinitely backwards-compatible, you can simply create a new one from scratch and only use it in new parser versions.

This is indeed quite an undertaking, in particular separating field formats means ironing out the field syntax, which is rife with inconsistencies. I however do not know if merely throwing out a proposal describing all this would yield any meaningful response, it all circles back to maintainer approval.

the constraints are missing?

make a non trival test pass

remove unused imports

make comment clearer
@gbaz
Copy link
Collaborator

gbaz commented Nov 13, 2023

As a cabal maintainer let me say I haven't given a careful enough look yet (in terms of reviewing all the related code and how it fits in) but the overall approach and argumentation for this proposal looks great, and I'm excited for it!

@gbaz
Copy link
Collaborator

gbaz commented Nov 13, 2023

by the way, regarding:

I think Cabal's current parsing approach is both incredibly unwieldy and not powerful enough (see Distribution.PackageDescription.FieldGrammar)

The comment linked to has nothing to do with the power of the parser and instead is about permissiveness of grammar with regards to back-compat for specific versioning and back-compat constraints. Its unrelated to anything under discussion.

andreabedini and others added 3 commits January 1, 2024 20:36
WIP

Add lexAll' to debug start codes

WIP: try to get the parser accept the new tokens

WIP
@jappeace jappeace changed the title Exact pp Exact printer May 5, 2024
jappeace and others added 3 commits June 8, 2024 14:48
WIP

Add lexAll' to debug start codes

WIP: try to get the parser accept the new tokens

WIP
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.

4 participants