MarkBind / markbind

MarkBind is a tool for generating content-heavy websites from source files in Markdown format
https://markbind.org/
MIT License
135 stars 124 forks source link

Debug cannot import footnote from hash #2441

Closed yiwen101 closed 6 months ago

yiwen101 commented 7 months ago

What is the purpose of this pull request? Resolve #2203 (but #2430 still persists)

Overview of changes:

Anything you'd like to highlight/discuss:

Testing instructions:

Proposed commit message: (wrap lines at 72 characters) Fix issue of cannot import footnote from hash


Checklist: :ballot_box_with_check:

currently, when importing from hash, footnote is missing. For example, with

<div id="import">

text^[footnote]
</div>

and

<include src="contents/topic1.md#import"/>

this is rendered

Screenshot 2024-03-04 at 03 30 29

but we expect

Screenshot 2024-03-04 at 03 28 23

This is fixed by using regular pattern to greedily extract and append the missing information.

Please note that #2430 still persists (the index of the imported footnotes may remain be conflicting). This PR only fixes only failure in importing footnote from hash, but did not touch the issue of wrong index.


Reviewer checklist:

Indicate the SEMVER impact of the PR:

At the end of the review, please label the PR with the appropriate label: r.Major, r.Minor, r.Patch.

Breaking change release note preparation (if applicable):

Give a brief explanation note about:

  • what was the old feature that was made obsolete
  • any replacement feature (if any), and
  • how the author should modify his website to migrate from the old feature to the replacement feature (if possible).
codecov[bot] commented 7 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 51.22%. Comparing base (371228d) to head (d555215).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #2441 +/- ## ========================================== + Coverage 49.04% 51.22% +2.17% ========================================== Files 124 124 Lines 5262 5281 +19 Branches 1118 1121 +3 ========================================== + Hits 2581 2705 +124 + Misses 2376 2290 -86 + Partials 305 286 -19 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

yiwen101 commented 7 months ago

Yay, thanks for the tests!

The issue #2203 also raises the example of the footnote in reader facing features. However, this is not fixed by your PR (is it because the new line isn't there?)

Can you also fix this documentation bug to demonstrate that this feature works?

Thanks for the comment; should be fixed in this commit

kaixin-hc commented 7 months ago

Behavior for footnotes here is definitely unexpected...

Screenshot 2024-03-04 at 3 23 04 PM Screenshot 2024-03-04 at 3 23 16 PM
yiwen101 commented 7 months ago

Behavior for footnotes here is definitely unexpected...

Screenshot 2024-03-04 at 3 23 04 PM Screenshot 2024-03-04 at 3 23 16 PM

emm, thanks for pointing out this (and sorry for not examining it closely);will be working on it.

yiwen101 commented 7 months ago

Thanks for the work @yiwen101! This is a rather tricky bug to fix so appreciate the work.

I have some concerns about the ordering of the footnote. I understand you aren't trying to address the wrong ordering here but I think this might result in overlaps - not just an issue with wrong ordering. Could you explain why you change the footnote number and why the appended footnote item will has the number 1-1?

Thanks!

Thanks for the careful review. The the number 1-1 is an unintended bug, thank you for pointing it out! The footnoteNumber is just a temporary placeHolder; the thinking is that when I/somebody attempt 2430 in the future and add footnoteNumber as an argument to this method, the rest of the code does not need to be changed (wait, just come to me, how to do passing by reference in javascript? nevermind, there must be a way to do it).

yiwen101 commented 6 months ago

@kaixin-hc @yucheng11122017 I have made the requested changes, and is ready for review; Summary of the changes: 1 improve the readability of the test 2 improve the code quality issue of hardcoded footnoteNumer = 1 3 fix bug. Some additional note on to the third point: Current behavior of serving user doc:

Screenshot 2024-03-14 at 16 18 22 Screenshot 2024-03-14 at 16 18 29

The bahaviour of the master branch

(after adding empty to enable the markdown:)

Screenshot 2024-03-14 at 16 15 47

(no pop up, when mouse is over; no response when clicked)

Screenshot 2024-03-14 at 16 14 54 Screenshot 2024-03-14 at 16 14 42