prql
prql copied to clipboard
PRQL code generation
This is an onboarding issue, see instructions on how to get started.
For debugging purposes and for the prql-compiler fmt
command, we have implemented a basic PRQL code generation from PL AST. It is done via impl std::fmt::Display for X
, which is convenient, but lacks a few key features:
- indentation can only be done ad-hoc, when outputting nested pipelines or lists,
- we have no control over maximum line width,
- we have no control on tab size.
Instead, I suggest we follow implementation of rustfmt. I've started work on codegen branch.
- start by implementing
WriteSource
forpl::StmtKind
. You should be able to reuse majority ofimpl Display for StmtKind
and adapt it to use string concatenation, similar to existing code insrc/codegen.rs
. For now, you can fallback to usingDisplay
(via.to_string()
) forFuncDef
. - uncomment
r += &stmt.kind.write(opt.clone()).unwrap();
incodegen.rs
andcargo run fmt a_test_file.prql
should be working
This is the gist of it. Similarly, we have adapt basically all Display
impls in ast::pl
to WriteSource
. I suggest starting with ExprKind
, which is already partially done and can easily be tested.
Because this is quite a lot of work, we can merge smaller commits implementing any small chunk of the codegen.
This issue is a prerequisite for #130.
@Rishav1707 this is the followup on #1418
FYI — in #2016 we test that the examples, after being formatted with the existing formatter, can still compile.
Where they can't, the language in the markdown in the book is prql_no_fmt
rather than prql
.
FYI I deleted a couple of messages to keep the issue clean, I confused things.
@aljazerzen I think for a Good "Actually First" Issue, I think this needs an initial effort for the framework. I feel like it's at my level at the moment, but not trivially so, and so for a completely new person it might be quite confusing.
- (Not saying you should necessarily do it, tbc. Instead just raising the issue — I've updated my view on the amount of context that a good "actually first" issue can require, and I think it should ideally be limited to a couple of files, if possible with a similar PR that's already been merged. So this could work really well after the first one)
I did the framework on codegen branch, as noted in the description.
It may need to be rebased to latest main branch. I didn't t merge it because we'd have to maintain it along with existing codegen impl.
(Thanks a lot. I added some thoughts to the broader point in #1840. Hope this isn't interpreted as criticism, am still trying to work out why we haven't been so successful here)