Prefer do instead of ' in pretty-printer

[pchiusano]

Before:

fork somewhere '(1 + 1)

After:

fork somewhere do 1 + 1

That is, we would previously avoid using do blah unless blah was a block rather than a single expression. This PR changes it to use do everywhere. It will add parens around the do if needed, as in foo (do 1 + 1) bar.

This is generally how I write code, it looks nicer to elide the parens in places where that's possible (and it allows more soft-hanging). You can always use (do 1 + 1) anywhere you were tempted to do '(1 + 1) previously.

This small change gets exercised pretty well by the existing transcripts and round-trip tests, but I'd like @stew or @ceedubs to try it out on nimbus codebase.

Maybe we put this behind a command line flag to start, so people can experiment with it? It's also easy to back out if we want, it was a small change.

[aryairani]

@pchiusano

I'm not sure what's going on with Windows stack, but can look into it later. (Though I wouldn't stop anyone else from looking into it.)

Re "trigger CI", you shouldn't have to; I meant to put a note on the "automatically run ormolu" commit that just says to refer to the previous commit's CI result.

Maybe we put this behind a command line flag to start

Do you want to, or wdyt?

Not sure what the status of this PR is; is it that we want to wait for someone to build and try it with nimbus for a bit, and then see if we want to add a command-line flag for it? If so, I would "Convert to draft" PR for now, otherwise it might get merged.

[pchiusano]

@aryairani I think putting it behind a flag is probably more work than I'd like to do on this PR, since that's going to have to be plumbed through to the pretty-printer which sounds like it could be annoying and touch a lot of stuff. I'm not opposed to someone else picking that up though.

So for now I'll convert to draft, and once @stew and @ceedubs have a chance to try it out, if it works okay and most people seem to prefer this style, we can merge it. It's easy to reverse if we want later, without breaking anything, so we probably don't need to be that conservative about it.

Also cc @SystemFw @runarorama @rlmark @alvaroc1 @dfreeman would love your opinion on this.

[SystemFw]

I'm in favour of using do pretty much anywhere, including (do ...) instead of '(...)

[pchiusano]

@SystemFw me too. Especially since my editor always wants to match ' characters and there's a way to turn it off but I keep forgetting to look into how. :)

[pchiusano]

Also I just came up with an amazing backronym.

do is short for "delayed operation".

🤣

[aryairani]

Should we go a step further and get rid of ' delay syntax altogether, freeing it up for something else? e.g. character literals

[ceedubs]

It's a bit of a pain for me to test this until #4794 is merged. Hopefully either @stew can or @pchiusano you could try to pull the latest nimbus and edit.namespace.

[pchiusano]

I just pulled nimbus and verified it round trips fine. (Except unrelated, edit.namespace dumps all the record accessor functions to the file and then UCM complains about duplicates - do we have a bug tracking this already? But once I removed all the duplicates everything round tripped with the same hash.)

And it seems like most people like the change, so I'd go ahead and merge.

[pchiusano]

We can consider removing ' in a separate PR. I probably wouldn't do that in a single release.

[aryairani]

Noting that the header on unison-src/transcripts-round-trip/main.md says we're just looking for the code to print and be parseable, not necessarily that it should print the same code that had been parsed. So to that extent, we're good.

[pchiusano]

@aryairani there are actually a couple checks - everything in reparses-with-same-hash.u reparses with an identical hash. And everything in reparses.u reparses with a different hash (there's currently just one thing in there).

And if this changes, the main transcript will fail.

[aryairani]

@pchiusano TIL