Allow to tune behavior of the brief description renderer

[smelc]

Context

First of all, thank you @HuwCampbell for your work on optparse-applicative. We (at cardano-cli) are very happy users of optparse-applicative!

This PR allows to tune the behavior of the renderer of descriptions. An option is added that favors vertical alignment when displaying multiple flags, instead of maximizing the use of space on a single line.

The default behavior is unchanged and no change is imposed upon users. This PR solely make it possible for callers that prefer vertical alignment to do so.

Caveats

I am not super savvy about the different options that could be proposed. I have only proposed the existing behavior and the new one, which happen to yield very different results, as the change to the golden file of the changed test exemplifies:

We have many more examples at cardano-cli where we find this change beneficial in terms of readibility of the --help message when there are many flags available. For example:

Here is another one:

History

For the record, this is an improvement that originally comes from #428 and which we have maintained in this fork, but we now want to remove this fork, to shrink the spread in the Haskell ecosystem, and reduce maintenance burden.

[HuwCampbell]

G'day,

So if memory servers hangAtIfOver becomes exactly align if the column is less than the 35 parameter, so it might be even simpler to tune that what you have.

I'll have a proper look and think soon, I'm not too opposed to this if folks would find it useful, though to be honest I think the default layout is nicer.

Cheers.

[smelc]

Nice to meet you @HuwCampbell and looking forward to your review 😎

[smelc]

Hey @HuwCampbell, I hope you're doing good 👋 Did you have the time to look at this PR?

[HuwCampbell]

G'day.

Yes, I am doing quite well. My family has a new puppy, they're very time consuming (both my kids and the puppy).

Honestly I'm a bit finicky about changes to the pretty printing because there's no one size that fits all. I just try to balance things as best I can to make our users' users experience as pleasant as possible.

Personally, I think that pushing everything over to the right that far and having alternatives split over different lines like you've shown is quite undesirable and should be avoided. The printer takes quite a bit of care to keep alternatives on the same line if it can. (This is the functions groupOrNestLine as part of foldTree over alternatives, I think it does this really well).

That said, I'm not going to unequivocally say no to a simple customisation if it's called for.

As far as this PR goes, I don't think this is quite the right thing. If I ever want to adjust said option it's a PVP breaking change right on the data type, and I try to do at most one a year at this stage.

So I would prefer just one integer option on the level at which you prefer to reïndent.

[HuwCampbell]

I think it would be better to do this differently, requiring the consumer of optparse to use the constructor and having it exported in types makes it a part of the "public API" which is a bit fragile; and it's not really in line with the other modifiers we have.

In my opinion it's better to instead have an option like:

briefHangPoint :: Int -> PrefsMod

which sets the location where the hang constructor kicks in (this is just 35 currently).

Now, when I mentioned that I thought the constructor was redundant, I was going off the definition of hangAtIfOver being

hangAtIfOver :: Int -> Int -> Doc -> Doc
hangAtIfOver i j d =
  column $ \k ->
    if k <= j then
      align d
    else
      linebreak <> ifAtRoot (indent i) d

so if one sets this to maxBound it is functionally identical to align.

So I think just not having that type is just as good an option.

What do you think?

[smelc]

Hey @HuwCampbell, I did the change and indeed it's simpler 👍

And I confirm that setting j to maxBound is indeed functionally equivalent to the previous design 👍

[HuwCampbell]

The point of this test was to show the behaviour of the hang feature, so if you could delete the second commit I'd be much obliged.

[smelc]

@HuwCampbell> I deleted the second commit and rebased on latest master 👍

Locally on my machine, all tests pass.

[HuwCampbell]

Ta