Simplify algebra rendering #14403
Simplify algebra rendering #14403
Conversation
| @typep mode :: :flat | :flat_no_break | :break | :break_no_flat | ||
|
|
||
| @spec fits( | ||
| @spec fits?( |
There was a problem hiding this comment.
It can now be a boolean because format injects a fake :group_over document that allows us to track where the parent groups stops. And by making next_break_fits, we can avoid look aheads.
There was a problem hiding this comment.
Since this changes the contract between the formatter and the algebra, is there a risk for this to be a breaking change for libraries relying on algebra? (e.g. now they need to add a fake |> group() too?
There was a problem hiding this comment.
I have added doc_fits back and I confirmed that the formatter without the changes in this PR still works, except for the next_break_fits bug you fixed (which caused this function to change). So if they want the fix, they need to move to the new API. :)
There was a problem hiding this comment.
I think that's fair, it was quite the edge-case bug 🙂
|
|
||
| defp size(%Date.Range{first_in_iso_days: first_days, last_in_iso_days: last_days, step: step}), | ||
| do: abs(div(last_days - first_days, step)) + 1 | ||
| defp size(%Date.Range{ |
There was a problem hiding this comment.
This was going over the line limit.
| @typep mode :: :flat | :flat_no_break | :break | :break_no_flat | ||
|
|
||
| @spec fits( | ||
| @spec fits?( |
There was a problem hiding this comment.
Since this changes the contract between the formatter and the algebra, is there a risk for this to be a breaking change for libraries relying on algebra? (e.g. now they need to add a fake |> group() too?
lib/elixir/lib/inspect/algebra.ex
Outdated
| will convert breaks into newlines, hoping the child group fits. | ||
| However, if the child group is optimistic, then the parent can | ||
| assume that it will fit, leaving the overall fitting decision | ||
| to the child |
There was a problem hiding this comment.
@sabiwara I pushed more docs here. Another way to call these options would be to call them "tie_breaker":
- parent
- child
- force_parent
But I think I like normal/optimistic/pessimistic better.
There was a problem hiding this comment.
Claude suggested normal/deferred/???, but could not come up with good names for the third option.
There was a problem hiding this comment.
Naming is hard...
Since it's "leaving the overall fitting decision to the child", something around the lines of delegate?
:normal | :delegate | :cancel_delegate? (just spitballing, I don't really like it)
There was a problem hiding this comment.
Yeah. I think the current one is my favorite. The optimistic also explains why force_unfit is ignored by the parent.
|
💚 💙 💜 💛 ❤️ |
Before we had
doc_fitsanddoc_groupwhich could be nested arbitrarily and complicated the formatting and fitting logic. By merging them into optimistic/pessimistic groups, we can simplify the API and logic.