Skip to content

implement tables - #52

Open
CrowdHailer wants to merge 6 commits into
lpil:mainfrom
CrowdHailer:tables
Open

CrowdHailer wants to merge 6 commits into
lpil:mainfrom
CrowdHailer:tables

Conversation

@CrowdHailer

Copy link
Copy Markdown
Contributor

Just used the available tests as mentioned in #36

Wraps up running the formatter. let me know if you want that on a separate commit or branch.

@CrowdHailer

Copy link
Copy Markdown
Contributor Author

Ah, I think I'm on the wrong gleam version that's what's making the reformat happen.

@lpil lpil left a comment •

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow, fantastic work! Thank you!!

I've added some notes inline, mostly around performance. This code will be in the hot path for lots of applications, so performance is really important.

I also pushed a commit that run the formatter.

Comment thread src/jot.gleam Resolved
Comment thread src/jot.gleam Resolved
Comment thread src/jot.gleam Resolved
Comment thread src/jot.gleam Resolved
@CrowdHailer

Copy link
Copy Markdown
Contributor Author

I've force pushed a new version. Have kept most function names but switch quite a few things.

  • TableAlignment is pulled at parsing time
  • Tried to make the caption code clearer.
  • Switched to use splitters.

@lpil lpil left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!! I've left a few questions and notes inline.

Comment thread src/jot.gleam Resolved
Comment thread src/jot.gleam Resolved
Comment thread src/jot.gleam Resolved
Comment thread src/jot.gleam Resolved
Comment thread src/jot.gleam Resolved
Comment thread src/jot.gleam Resolved
Comment thread src/jot.gleam Resolved
Comment thread src/jot.gleam Resolved
Comment thread src/jot.gleam Resolved
Comment thread src/jot.gleam Resolved
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

Successfully merging this pull request may close these issues.

2 participants