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.

Sign up for GitHub

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

Jump to bottom

Replacing/Breaking off Text.Tabular.AsciiWide. #1679

Open
Xitian9 opened this issue Aug 31, 2021 · 2 comments · May be fixed by #1850
Open

Replacing/Breaking off Text.Tabular.AsciiWide. #1679

Xitian9 opened this issue Aug 31, 2021 · 2 comments · May be fixed by #1850
Labels
A-WISH Some kind of improvement request, hare-brained proposal, or plea.

Comments

@Xitian9
Copy link
Collaborator

Xitian9 commented Aug 31, 2021

There have been calls at various points to break off AsciiWide and WideString into its own library (see e.g. #1136).
The functionality is completely independent of hledger, and making it available for re-use/decoupling the versions would be useful. AsciiWide also happens to add a surprising amount to build time (a little under 5s on my system), so splitting this off would help build times.

That said, AsciiWide may not be (is probably not) the best solution to the problem, and instead of reinventing the wheel maybe we want to adopt another solution. table-layout is a solution that people here have looked at before, and seems well-aligned to the job to be done. It also seems that it would make aligning on decimal points (a long-requested feature) easy. However, it does not seem to support wide characters (muesli4/table-layout#8). @ony and @simonmichael put in some effort to see if that could be added about a year and a half ago; maybe I could put out feelers to see if they are open to more work on that.
@Xitian9 Xitian9 added the A-WISH Some kind of improvement request, hare-brained proposal, or plea. label Aug 31, 2021
@Xitian9
Copy link
Collaborator Author

Xitian9 commented Sep 3, 2021 •
edited

I've done a bit of looking in to table-layout, and there are a few questions we would need to answer before deciding whether migration is suitable.

Blockers

  • Wide character support It is claimed that although table-layout doesn't support this natively, the typeclass-based interface allows it to be implemented easily. Edit: I've submitted a PR to add this functionality to table-layout, but it could easily be implemented in hledger as well.
  • Multiple colours in a cell We need to be able to colour different amounts in a table differently. table-layout currently only supports a single colour within a cell, but it shouldn't be too difficult to work around this. Edit: I've submitted a PR to add this functionality to table-layout, but it could easily be implemented in hledger as well.
  • Not currently on stackage

Non-blockers, but significant formatting differences

  • table-layout currently does not support different separators for different rows/columns. For example, we use double-lines to separate the leftmost column from the others, and then no line to separate further columns.
  • table-layout currently does not support separators of more than a single character, like the double pipes and pluses ||/++ that we use for our tables. Edit: I've submitted a PR to add this functionality to table-layout.
  • table-layout currently always includes an outer border, while we never do. Edit: The same PR enabling ||/++ would also enable this functionality.

If we were willing to accept a big change in how our tables were displayed then we could ignore any of these non-blockers. However, that would be a huge pain for upgrading the test suite, plus I like our current style.

A decision needs to be made on whether pursuing a migration would be acceptable if any of these issues are unable to be resolved (because table-layout doesn't want to or can't implement the feature).

@muesli4
Copy link

muesli4 commented Sep 11, 2021 •
edited

table-layout currently does not support different separators for different rows/columns. For example, we use double-lines to separate the leftmost column from the others, and then no line to separate further columns.

I can see an easy way to implement this with styles and some help of type classes. If you're interested you can create an issue. I'm definitely willing to improve the usability.

@Xitian9 Xitian9 linked a pull request Mar 30, 2022 that will close this issue
Draft
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-WISH Some kind of improvement request, hare-brained proposal, or plea.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Switch to table-layout for tabular layout Xitian9/hledger
2 participants
@muesli4 @Xitian9