Skip to content

Footnotes improvements #1218

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

Merged
merged 5 commits into from
May 5, 2022
Merged

Conversation

ysard
Copy link
Contributor

@ysard ysard commented Feb 3, 2022

Hi, I suggest in this PR 2 improvements for the footnotes module.

  • Small refactor of the BACKLINK_TITLE option; The use of format() instead of "old" %d formatter allows
    to specify text without the need to have the number of the footnote in it (like footnotes on Wikipedia for example).
    The modification is backward compatible so no configuration change is required.

  • Addition of a new option SUPERSCRIPT_TEXT that allows to specify a custom placeholder for the footnote itself in the text.
    Ex: [1] or (1) or just by default, the current behavior: 1.

Some writing rules specify these subtleties I think it could be useful.

Thanks for reading.

ysard added 2 commits February 3, 2022 03:41
- The placeholder '{}' is optional. So a user can choose to include or
not the footnote number in the backlink text.
- The modification is backward compatible with configurations using
the old '%d' placeholder.
- The addition of a new SUPERSCRIPT_TEXT option allows to specify
a placeholder receiving the footnote number for the superscript text.
@waylan
Copy link
Member

waylan commented Feb 3, 2022

These look like some good improvements/changes to me. Of course, we do need to include an update to the changelog. However, as this is adding a new feature, it will need to be in a new feature release, which means a new document will need to be added for the release notes. However, we already have PR #1187 which handles that. This can be finalized and merged after that.

@waylan waylan added extension Related to one or more of the included extensions. feature Feature request. requires-changes Awaiting updates after a review. labels Feb 3, 2022
@waylan
Copy link
Member

waylan commented Feb 3, 2022

Oh, I just realized there are no tests for the new features. We also need some new tests added to tests/test_syntax/extensions/test_footnotes.py.

@ysard
Copy link
Contributor Author

ysard commented Feb 3, 2022

Thanks for the review, tests are added and passing (Ran 945 tests in 3.294s, OK (skipped=21)). I also added the changelog hoping that it will make your work easier after the merge for 3.4.

@waylan waylan added approved The pull request is ready to be merged. and removed requires-changes Awaiting updates after a review. labels Feb 4, 2022
@waylan waylan added this to the Version 3.4 milestone Feb 25, 2022
@waylan waylan merged commit efec51a into Python-Markdown:master May 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved The pull request is ready to be merged. extension Related to one or more of the included extensions. feature Feature request.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants