Actually (as I guess you have seen) in all default cases, out_indent defaults to display_indent, which itself defaults to display_blanks, and out_spaces defaults to display_blanks. So in a default setting they behave the same.

Regarding code breaking, we scanned opam packages for this PR and identified the ones which could break (for example I sent a patch for utop).

The first case was discussed in the PR (or one of the related ones). Regarding the second, I think it was not. Do you have a lot of examples of 2. in your codebase that change out_spaces ? (if so, basically I am ok with your proposal, since we missed this implication).

All in all it is indeed true that the changes are not retro-compatible.

The only problem that I see with this change is that, going forward, if you change out_spaces you could inadvertently change out_indent as well without knowing. In effect, this patch favors retro-compatibility over clear separation of concepts in the future. I think any choice will impact only a small handful of power-users which should be able to manage the technical implications, as long as they're well documented.

That's why I think we should mention the side-effect you propose in the documentation of the module, if we go forward with the patch. We should even say that you should change out_indent whenever you redefine out_spaces if your intent is not to change both at the same time.

Other than that I would be ok with the patch for the sake of retro-compatibility, especially if some codebases (not in opam) have a lot of {... with } cases redefining out_spaces.

If the patch is not accepted, I suggest we add to the documentation the pitfalls you mention.

Thanks for the comments @rbonichon!

I only had to adapt one instance of "2.", but I found the non-backwards compatible behaviour surprising, specially because the documentation currently reads:

field [out_indent] is the same as field [out_spaces].

which I initially interpreted to mean the code in this PR (meaning that updating out_spaces also changes out_indent)!

@rbonichon thanks for your enlightening analysis. I found this bit in particular interesting:

If you change out_spaces you could inadvertently change out_indent as well without knowing. In effect, this patch favors retro-compatibility over clear separation of concepts in the future.

I am not very familiar with the use-cases for these functions. Could someone explain what are the typical use-cases for overriding out_spaces and out_indent? Is it actually the case that most changes to "spaces" should naturally also affect the notion of identation (which seems to be @nojb's assumption)?

@gasche Just to give an example of a possible use: to define a kind of formatter that prints everything in compact fashion (in one line with the minimum number of whitespace), one would override out_spaces (and now also out_indent) to output at most one ' ' character.

As for the expectation that most changes to "spaces" should also affect "indentation", that has been the case until now (where "indentation" was not a separate concept from "spaces") and I was just arguing purely from the point of view of backwards compatibility.

@gasche Prior to #615 , out_spaces was used to input simple typographic spaces and indenting by n was in effect just printing n spaces, since there was no out_indent. However, indenting seems (to me and @pierreweis) to be a different concept: that's why we chose to add one more field to the record construct when we added symbolic formatting functions.

Overriding out_ident (by default it prints out simple spaces) could make sense if you print out a language where you indent with tabs (say a Makefile) through Format.
Now overriding out_spaces could make sense if you want to explicitly display the spacing in your pretty printer (as the way some word processor show formatting marks).

Actually, @pierreweis and I did exactly that (display formatting marks) when we were preparing examples for our paper on Format last year. That's actually when it struck us that there was no clean way of graphically differentiating indentation and spacing, which ultimately lead to the introduction of out_indent.

I'll let the community decide if backwards compatibility is more important than a cleaner (from my point-of-view) separation between out_indent and out_spaces, which might break some programs. Whichever way we go, we need to enhance the documentation to reflect the discussion started by @nojb here.

Since #615 was not in 4.05, "the community" should decide before the 4.06.0 release.

Let's assume we don't include the current PR in the release -- that's my reading of the fairly-weak consensus around. Could someone propose a documentation PR about the issue fairly soon? (Ideally we would like to release before the end of the month.)

I agree that the "consensus" seems to be to leave things as they are, which is fine by me. I just pushed a tiny one-line change to the documentation that would have cleared things up for me when reading it.

Merged, cherry-picked in 4.06: 9f4e707

@Octachron, if it's easy for you to update your manual beta, that could be an occasion.

Done: http://www.polychoron.fr/ocaml-beta-manual/4.06/libref/Format.html#TYPEformatter_out_functions

@nojb Thanks for the added documentation. I'll try to add a line to the Changelog reporting what you need to do in case you want a behavior compatible with the previous release.

One of the packages that breaks because of the new record field is notty : see pqwy/notty#17. I'm linking the issue here for reference. It seems from the discussion that notty's author, @pqwy, would have welcome @nojb's initial transitional proposal.

I propose a fix for this second case by making the default out_indent reference out_spaces, so that any update to out_spaces will also affect out_indent.

Notty's author strongly agrees.

(surprised that this was not done in the original PR, I wonder if I am missing something obvious...)

I'm also surprised at this gratuitous breakage, since it's so easy to mitigate.

It's a debate between retro-compatibility vs. having to deal with just as surprising behaviors in the future -- indeed if you start OCaml at 4.06, why would you expect that changing out_spaces would affect out_indent since they are two different fields ?

Also we identifed only 2 packages (now 3) at the time of the PR, so code breaking objectively seems to be pretty rare. Due to lack of strong consensus, we went with more documentation in the Changelog.