alda-lang / alda

A music programming language for musicians. :notes:
https://alda.io
Eclipse Public License 2.0
5.61k stars 288 forks source link

Alda Code Formatter #474

Closed Scowluga closed 1 year ago

Scowluga commented 1 year ago

Background

Currently for MusicXML import, we are able to go from MusicXML to Alda []ScoreUpdate. The following 3 PRs combined will introduce an Alda code generator to complete the import process:

  1. https://github.com/alda-lang/alda/pull/471
  2. https://github.com/alda-lang/alda/pull/474
  3. https://github.com/alda-lang/alda/pull/475

This PR introduces a code formatter, translating ASTNode to Alda code.

func FormatASTToCode(root ASTNode, out io.Writer, opts ...formatterOption) error {

Notes

Whitespaces

The formatter is quite rudimentary, but has basic handling of whitespacing (indents, spaces, line wrapping) while maintaining the integrity of the generated code.

Variable Definitions

Variable definitions are tricky as I've noted in code comments. As a summary, the problem is all definition nodes must be on the same line as the variable name. The easiest way I found to handle this is to introduce the varDefState enum, which introduces some significant complexity into the formatter.

Testing

We piggyback and use the parseTestCase to also automatically test formatting. Like with ast-gen, we ensure the "round-trip" formatted then parsed AST is identical to the actual correct AST.

Comments

Currently comments are dropped when scanning, so they will be dropped for a round-trip format (code -> AST -> code).

I've spent a couple hours trying my hand at the changes necessary to retain comments to the AST level. I think this change is not worth it, as it's quite complicated and touches sensitive code. Plus MusicXML import doesn't produce comments, so the change is not necessary for this main use case. Plus the formatter itself is not very good, so I don't think it'll really standalone.

I've left supporting comments as a TODO.

Commands

The formatter is not quite good enough in my opinion to standalone as it's own command to actually use. Combined with the fact that we don't even support comments, we will introduce an experimental hidden alda format command.

daveyarwood commented 1 year ago

@Scowluga I think this PR is just about ready to merge -- let's just rename the --output flag to --overwrite