NB: this triggers some errors in some of the dune files, I intend to fix these.

Can we maintain the standard lisp formatting? Where trailing parens are on the same line. I don't see a reason to break with tradition here.

I like the output, but I agree with @rgrinberg about trailing parentheses. If we don't group them they occupy a lot of lines.

It would be nice if we could also preserve the original formatting of strings. Could you also add a mode where it just reformat all the dune files in the source tree in-place?

I don't have a lot of lisp experience, so I tried to keep this as simple as possible for now. Is something like this more idiomatic?

(rule
 (progn
  (run true)
  (with-stdout-to
   x
   (echo b))))

It would be nice if we could also preserve the original formatting of strings.

I believe that this will require an alternative parser, so I suggest that I do this in a separate PR (with comments as well).

Could you also add a mode where it just reformat all the dune files in the source tree in-place?

Sure!

I don't see a reason to break with tradition here.

We can probably make two (related) arguments: it diffs poorly, and it's difficult to edit the end of a s-expression, for example to add a field to (library) or the like.

Personally, i don't mind about breaking the tradition, all that matters is that it looks good. It's true that it might be annoying if it's hard to add a field at the end.

Do you know how we can integrate this with emacs? so that for example the file is automatically reformatted when saving the file to disk. I'd like to try to see how it feels.

We can probably make two (related) arguments: it diffs poorly,

What's the issue with diffing? It works quite well for me. Even better is the structural diff that is available in Emacs.

and it's difficult to edit the end of a s-expression, for example to add a field to (library) or the like.

With what editor? In emacs this is 1 key combination. In vim it shouldn't be very hard either. Just go to the last field, hit %i<return> and write.

Here's another point for the classical formatting: almost everyone uses it. The vast majority of jbuild files use it. You can check it yourself with cat $(find . -name jbuild) | less if you don't believe me. That signals preference towards the classical style.

I don't want this PR to turn into a debate on what should be the definitive syntax. The point is to start from something easy to implement and then we can improve based on feedback.

Even better is the structural diff that is available in Emacs.

Emacs is very powerful to edit s-expressions, but not all dune users use it.

IMO having ) characters more visible will make dune easier to use for people less used to s-expressions.

Here's another point for the classical formatting: almost everyone uses it. The vast majority of jbuild files use it. You can check it yourself with cat $(find . -name jbuild) | less if you don't believe me. That signals preference towards the classical style.

I believe you, but this is a bit biased. For example, when contributing my first dune files I checked in this repo to see how this was already done here, and adjusted. But for fresh projects, I used the the more vertical style like this one (see also this one).

Do you know how we can integrate this with emacs? so that for example the file is automatically reformatted when saving the file to disk. I'd like to try to see how it feels.

I'm not familiar enough with elisp to provide a working example, but something like this should work:

; warning: untested
(defun dunefmt-before-save ()
  (interactive)
  (shell-command-on-region
   (point-min) (point-max)
   "dune fmt --unstable"))

(add-hook 'before-save-hook #'dunefmt-before-save))

I agree, let's not discuss this eagerly. We need to try this to see what's best in practice, so merging this as an unstable feature that we can play with seems good.

Okay, we can leave the syntax aside. I still think its dubious to include the formatting tool if we can't even use it on our dune files.

Btw, do we really need this --unstable flag? I'd like to see some motivation for it as I think it's just bad UI to have a command that only works with a flag. We might as well rename the command to $ dune unstable-fmt.

Btw, do we really need this --unstable flag?

The idea was to just remove that flag at the end rather than rename the command, but it's the same. I followed your suggestion and renamed the command.

It is a nice addition. A syntax related comment (I don't know where you want to keep them). The following seems not right. Instead of the hard rules you used for when line break should appear, can't you use a more dynamic box-like approach (e.g. Format? ) so that this is on one line?

  (with-stdout-to
   x
   (echo b)
  )

Thanks for the feedback! Can you open a separate issue for this?

Done in #1153.