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

more principled generic printer #533

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from
Draft

more principled generic printer #533

wants to merge 6 commits into from

Conversation

W95Psp
Copy link
Collaborator

@W95Psp W95Psp commented Feb 27, 2024

Exploring more principled/clean generic printer after chatting about the PV backend in Paris with Jonas.
Porting the PV printer should be easy, but that's a bit out of scope, I'm trying to play with the generic printer to converge to something nicer.

This generic printer supports fully source maps v3: any backend using the genric printer gets that for free.

The result is something like that or the fact a user can click in VSCode in some region of the extracted code and be directed to the Rust source.

Fixes #404

@W95Psp
Copy link
Collaborator Author

W95Psp commented Jul 1, 2024

I updated this PR in a way the current generic printer and the new one cohabit.
We need to move PV to this generic printer (see #745), and then we will be able to remove the previous generic printer.

@W95Psp W95Psp marked this pull request as ready for review July 1, 2024 09:09
@W95Psp W95Psp added the waiting-on-reviewer Status: Awaiting review from the assignee but also interested parties. label Jul 2, 2024
Copy link
Member

@franziskuskiefer franziskuskiefer left a comment

Choose a reason for hiding this comment

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

Let's get some documentation in before we merge this.

))
})
});
Copy link
Member

Choose a reason for hiding this comment

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

As usual, some comments on what the new code is doing would be nice.

@@ -0,0 +1,92 @@
open! Prelude
Copy link
Member

Choose a reason for hiding this comment

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

Not sure if the name is the best ....
But aside from that, we need a design of this. Please add a design document (markdown, mermaind, whatever) that describes how this printer works and how it can be used.
And then document the code (with how it's implementing the design).

Copy link
Member

Choose a reason for hiding this comment

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

remove?

@franziskuskiefer franziskuskiefer removed the waiting-on-reviewer Status: Awaiting review from the assignee but also interested parties. label Aug 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Status: No status
Development

Successfully merging this pull request may close these issues.

Generic printer: optionally output source maps
2 participants