How to make brittany format data declarations? #47

New Issue

2017-08-14T20:01:32+02:00

eschnett commented

2017-08-14 20:01:32 +02:00

(Migrated from github.com)

I'm thinking of what it would take to make brittany format data declarations as well. I've looked at the high-level descriptions and have skimmed the source code.

The source code is a bit difficult to navigate since I don't know which files or directories have what function. There also don't seem to be comments indicating which file / function / case branch relates to what kind of language elements.

My first attempt at extending brittany would be to copy-past some existing code, re-using e.g. let or case expressions to format data statements, and making this work in simple cases. The errors and misfortunes I'd encounter would help me learn more about brittany, in the best traditions of cargo-cult programming. Would this stand some chance of success?

I'm thinking of what it would take to make brittany format data declarations as well. I've looked at the high-level descriptions and have skimmed the source code. The source code is a bit difficult to navigate since I don't know which files or directories have what function. There also don't seem to be comments indicating which file / function / case branch relates to what kind of language elements. My first attempt at extending brittany would be to copy-past some existing code, re-using e.g. `let` or `case` expressions to format `data` statements, and making this work in simple cases. The errors and misfortunes I'd encounter would help me learn more about brittany, in the best traditions of cargo-cult programming. Would this stand some chance of success?

👍 5

lspitzner commented

2017-08-14 20:38:31 +02:00

(Migrated from github.com)

You probably have not noticed the branch "datadecl" yet; it already contains some (unfinished) initial work in this direction. Although I notice I have a commit that is not even pushed on that branch.

Let me rebase and push, one sec.

You probably have not noticed the branch "datadecl" yet; it already contains some (unfinished) initial work in this direction. Although I notice I have a commit that is not even pushed on that branch. Let me rebase and push, one sec.

lspitzner commented

2017-08-14 21:13:49 +02:00

(Migrated from github.com)

Ok, pushed.

It is good news that you want to contribute and I should be able to give you some more specific hints at how to approach the task.

Ok, pushed. It is good news that you want to contribute and I should be able to give you some more specific hints at how to approach the task.

lspitzner commented

2017-08-14 21:35:43 +02:00

(Migrated from github.com)

Firstly, my workflow in general when enhancing brittany is:

Think of some short example code that contains the not-yet-layouted syntax;
pass that to brittany --dump-ast-full
Find out the relevant parts of the GHC API. In this case, TyClD - don't let the weird naming confuse you, data decls are a form of Ty(pe)Cl(ass), apparently.. - and then DataDecl
Add some new pattern match or make a new one (or create a new file even) - this is already done on the branch.
start guessing at what the fields of the GHC API datatype mean
Write the actual layouter code. One can insert dummies such as docLit (Text.pack "TODO node XY") as a first sub-step
pass your initial example to a newly-built brittany and find out exactly how broken the output still is. Repeat steps 6/7.
Write tests that have ~~minimal~~ 100% coverage

This is a quick iterative approach; A more thorough one would be test-driven: First, think of all the different relevant cases (here: newtype simple, newtype record, data with one constructor, data with one record, data with multiple constructors, data with multiple record constructors, GADT, (and i still have not investigated if there are GADT records..)). Write (a/ simple example/s) for each case, and consider how they should be layouted. Only then start looking at the relevant GHC API nodes.

Firstly, my workflow in general when enhancing brittany is: 1) Think of some short example code that contains the not-yet-layouted syntax; 2) pass that to `brittany --dump-ast-full` 3) Find out the relevant parts of the GHC API. In this case, [`TyClD`](https://downloads.haskell.org/~ghc/8.0.2/docs/html/libraries/ghc-8.0.2/HsDecls.html#v:TyClD) - don't let the weird naming confuse you, data decls are a form of Ty(pe)Cl(ass), apparently.. - and then [`DataDecl`](https://downloads.haskell.org/~ghc/8.0.2/docs/html/libraries/ghc-8.0.2/HsDecls.html#v:DataDecl) 4) Add some new pattern match or make a new one (or create a new file even) - this is already done on the branch. 5) start guessing at what the fields of the GHC API datatype mean 6) Write the actual layouter code. One can insert dummies such as `docLit (Text.pack "TODO node XY")` as a first sub-step 7) pass your initial example to a newly-built `brittany` and find out exactly how broken the output still is. Repeat steps 6/7. 8) Write tests that have ~~minimal~~ 100% coverage This is a quick iterative approach; A more thorough one would be test-driven: First, think of all the different relevant cases (here: newtype simple, newtype record, data with one constructor, data with one record, data with multiple constructors, data with multiple record constructors, GADT, (and i still have not investigated if there are GADT records..)). Write (a/ simple example/s) for each case, and consider how they should be layouted. Only then start looking at the relevant GHC API nodes.

lspitzner commented

2017-08-14 21:59:26 +02:00

(Migrated from github.com)

Some other random thoughts:

regarding reading the GHC API: In this context, all "PostRn" stuff can be ignored.
master now works on both ghc-8.0 and 8.2, but of course that is not yet necessary for the additions; The difference in the API seems relatively small, so it should be no problem for me to adapt the new code to the other version later. I cannot think of arguments to start with one of them over the other, either.
Regarding handling of comments: Feel free to ignore for now. How to make them work is not documented, and looking at existing comment-handling-code will probably not give a clear view either. (My approach usually is to just pepper some docWrapNode applications, but that does not cover all cases and .. nevermind for now.)
I really recommend automating parts of that workflow. For example I usually have one simple Sample.hs that contains nothing but the example I am working on, and I have a bash script that looks something like
```
cabal build exe:brittany
./dist/build/brittany/brittany --omit-output-check --dump-annotations --dump-ast-full --dump-bridoc-final --output-on-errors -i ../local/Sample.hs "$@" +RTS -s -M2G 1> out.hs 2> err.txt
```
So I can quickly get the full new results. (For the compilation I recommend ghcid for quick feedback from ghc. Just mentioning; I don't know how much dev you do in haskell.)

Some other random thoughts: - regarding reading the GHC API: In this context, all "PostRn" stuff can be ignored. - master now works on both ghc-8.0 and 8.2, but of course that is not yet necessary for the additions; The difference in the API seems relatively small, so it should be no problem for me to adapt the new code to the other version later. I cannot think of arguments to start with one of them over the other, either. - Regarding handling of comments: Feel free to ignore for now. How to make them work is not documented, and looking at existing comment-handling-code will probably not give a clear view either. (My approach usually is to just pepper some `docWrapNode` applications, but that does not cover all cases and .. nevermind for now.) - I really recommend automating parts of that workflow. For example I usually have one simple `Sample.hs` that contains nothing but the example I am working on, and I have a bash script that looks something like ~~~~ cabal build exe:brittany ./dist/build/brittany/brittany --omit-output-check --dump-annotations --dump-ast-full --dump-bridoc-final --output-on-errors -i ../local/Sample.hs "$@" +RTS -s -M2G 1> out.hs 2> err.txt ~~~~ So I can quickly get the full new results. (For the compilation I recommend ghcid for quick feedback from ghc. Just mentioning; I don't know how much dev you do in haskell.)

lspitzner commented

2017-08-14 22:09:34 +02:00

(Migrated from github.com)

Oh, and to answer the initial question: It should not be necessary to touch any code other than Language.Haskell.Brittany.Internal.Layouters.* when adding support for other parts of ghc syntax. The structure of these modules roughly matched the structure of the GHC API (Layouters.Expr ~ ghc:HsExpr, Layouters.Pattern ~ ghc:HsPat etc.) although the new branch deviates as DataDecl is in HsDecls - but I really prefer to split up stuff a bit, the modules are big enough as they are.

Oh, and to answer the initial question: It should not be necessary to touch any code other than `Language.Haskell.Brittany.Internal.Layouters.*` when adding support for other parts of ghc syntax. The structure of these modules roughly matched the structure of the GHC API (`Layouters.Expr ~ ghc:HsExpr`, `Layouters.Pattern ~ ghc:HsPat` etc.) although the new branch deviates as DataDecl is in `HsDecls` - but I really prefer to split up stuff a bit, the modules are big enough as they are.

lspitzner commented

2017-08-14 22:21:03 +02:00

(Migrated from github.com)

Thinking some more, perhaps it is not even good that the initial work is already done - to get familiar with the interface it might be better to start by deleting the current body of layoutDataDecl, and just play around with filling that hole for a bit. I am mildly optimistic that just creative copy-pasting from other parts will indeed be possible, and even if you move a few steps back, getting familiar with the brittany and GHC API should be the first goal.

Thinking some more, perhaps it is not even good that the initial work is already done - to get familiar with the interface it might be better to start by deleting the current body of `layoutDataDecl`, and just play around with filling that hole for a bit. I am mildly optimistic that just creative copy-pasting from other parts will indeed be possible, and even if you move a few steps back, getting familiar with the brittany and GHC API should be the first goal.

eborden commented

2017-12-21 16:00:34 +01:00

(Migrated from github.com)

@lspitzner is datadecl still a viable branch?

@lspitzner is `datadecl` still a viable branch?

lspitzner commented

2017-12-22 15:30:14 +01:00

(Migrated from github.com)

@eborden yes. i just rebased it to current dev. it only is 8.0 but doesn't seem to have bitrotten yet.

@eborden yes. i just rebased it to current `dev`. it only is 8.0 but doesn't seem to have bitrotten yet.

👍 1

tfausak commented