Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor lists formatting, make custom lists constructible with BlockTextBuilder #238

Closed
Tracked by #240
KillyMXI opened this issue Oct 5, 2021 · 1 comment
Closed
Tracked by #240

Comments

@KillyMXI
Copy link
Member

KillyMXI commented Oct 5, 2021

An idea formed after discussion in #231

Currently table construction is handled with the primitives provided by BlockTextBuilder. Formatter walks through the model of an HTML table and calls the builder to construct the output table.

Contrary, ordered and unordered lists completely handled in the formatter, only block primitives are called.

  • Pros: smaller API of BlockTextBuilder;
  • Cons: no reusable primitives to construct nice custom lists. To achieve the same quality formatting will require to copy the whole list-formatting code.

Solution: add extra functions to BlockTextBuilder that will take some responsibilities of

function formatList (elem, walk, builder, formatOptions, nextPrefixCallback) {

thus reducing the amount of formatter-specific code and increasing the amount of reusable builder code.

API variant 1:

builder.openList(/* { maxPrefixLength: number, prefixAlign: 'left' } */);
builder.openListItem({ prefix: string });
walk(...);
builder.closeListItem();
builder.closeList();
  • Similar to the API for tables;
  • Might not work with lists. Currently, all list item nodes are collected until all prefixes are known and walked only after that. Table cells have different requirements for text width, and rendered text is accumulated instead;
  • A compromise solution might be to leave it to client code to determine the max length of prefixes.

API variant 2:

builder.openList();
builder.addListItem({ prefix: string, elem: ... });
// or
builder.addListItem({ prefix: string, walk: () => walk(...) });
builder.closeList();
  • Can transfer the logic from existing formatter code;
  • Inconsistent with table building.

I'll need to review the table building, including WIP, to see whether it makes sense to change the API for tables. WIP code for tables wasn't going smooth, would be nice to find an improvement on both fronts.


I'm going with variant 1 and leave upfront max prefix length calculation in the formatter.
This provides most natural transition...

@KillyMXI
Copy link
Member Author

KillyMXI commented Dec 2, 2022

Now public with version 9.

@KillyMXI KillyMXI closed this as completed Dec 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant