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

add support for unordered list #44

Merged
merged 4 commits into from
Sep 20, 2024
Merged

add support for unordered list #44

merged 4 commits into from
Sep 20, 2024

Conversation

mxple
Copy link
Contributor

@mxple mxple commented Sep 19, 2024

No description provided.

@codecov-commenter
Copy link

codecov-commenter commented Sep 19, 2024

Codecov Report

Attention: Patch coverage is 0% with 10 lines in your changes missing coverage. Please review.

Project coverage is 0.00%. Comparing base (2cde70a) to head (3e4f302).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
tui-markdown/src/lib.rs 0.00% 10 Missing ⚠️
Additional details and impacted files
@@          Coverage Diff          @@
##            main     #44   +/-   ##
=====================================
  Coverage   0.00%   0.00%           
=====================================
  Files          7       7           
  Lines        489     492    +3     
=====================================
- Misses       489     492    +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Owner

@joshka joshka left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. It has some problems, but it's better than what was there before.

Given this input (from TEST.md):

image

This produces:

image

Input

- Item 1
- Item 2 (currently buggy)
  - Item 2a
  - Item 2b

Output

- Item 1
- Item 2a
- Item 2b

Which is better than what was there already.

I refactored this change to use None instead of 0 as the indicator of an unordered list in the list_index stack and will push it to your branch. If you git pull your branch, you can bring that change to your local and continue working on this.

@mxple
Copy link
Contributor Author

mxple commented Sep 19, 2024

I believe the bug is fixed. It's slightly hacky but I found that for nested lists, the item's end tag is emitted after the nested list block, not before.

@joshka
Copy link
Owner

joshka commented Sep 20, 2024

I believe the bug is fixed. It's slightly hacky but I found that for nested lists, the item's end tag is emitted after the nested list block, not before.

Thanks for fixing this. It makes sense to me why it was broken now.

In #45, I'm in the process of refactoring this to be a bit better about how the temporary line is handled. Instead of keeping a line field in the TextWriter struct, I just use the last line of the text field. This means that the actual text of any block (e.g. this list item) is directly added to the end result when encountered without having to wait and only add it when a new block is encountered. This seems to be a reasonable approach, and I hope there aren't any annoying edge cases where this doesn't work.

The PR is work in progress, so there's parts which are fully commented out / deleted that are not yet working. Maybe take a look and see whether you think this approach would make this less hacky?

@joshka joshka merged commit d332442 into joshka:main Sep 20, 2024
11 checks passed
@github-actions github-actions bot mentioned this pull request Sep 19, 2024
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.

3 participants