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

Fix markdown "break on newline" configurability #236

Merged
merged 8 commits into from
Nov 29, 2024

Conversation

threema-danilo
Copy link
Contributor

Previously, "break-on-newline": True was part of the default markdown options. The user could override the configuration value, thus turning it into "break-on-newline": False, but unfortunately the markdown2 library ignores the value of the option, and always sets it to True if the key is passed in.

The break-on-newline extra is deprecated, there is a new option since 2.4.11: breaks https://github.com/trentm/python-markdown2/wiki/breaks Unfortunately, markdown2 overrides the breaks.on_newline configuration if break-on-newline is present in the extras, meaning that this doesn't work for disabling "break on newline" either.

This means that right now there is no way for the users of jsfh to disable line breaks on newlines. To fix this, remove break-on-newline from the default markdown options. If (and only if) the breaks config is not set by the user, a breaks config is added to the defaults that inserts line breaks on newlines by default.

Previously, `"break-on-newline": True` was part of the default markdown
options. The user could override the configuration value, thus turning
it into `"break-on-newline": False`, but unfortunately the markdown2
library ignores the value of the option, and always sets it to `True` if
the key is passed in.

The `break-on-newline` extra is deprecated, there is a new option since
2.4.11: `breaks` https://github.com/trentm/python-markdown2/wiki/breaks
Unfortunately, markdown2 overrides the `breaks.on_newline` configuration
if `break-on-newline` is present in the extras, meaning that this
doesn't work for disabling "break on newline" either.

This means that right now there is _no_ way for the users of jsfh to
disable line breaks on newlines. To fix this, remove `break-on-newline`
from the default markdown options. If (and only if) the `breaks` config
is _not_ set by the user, a `breaks` config is added to the defaults
that inserts line breaks on newlines by default.
@threema-danilo
Copy link
Contributor Author

threema-danilo commented Mar 20, 2024

(As a sidenote, the markdown2 library seems quite convoluted to me, with regards to the parsing of options... Have you considered moving to a standards-conforming library like commonmark?)

@estan
Copy link
Contributor

estan commented Nov 28, 2024

I'd love to see this get it so that we can disable break on newline. Is there more work to be done here @dblanchette ?

@estan
Copy link
Contributor

estan commented Nov 28, 2024

@threema-danilo I think it would also be good to update the default in config_schema.json, perhaps with a comment about the conditional-ness.

@threema-danilo
Copy link
Contributor Author

@estan thanks for the update! I don't currently have the free capacity to work on this issue, but feel free to push changes to my branch in order for it to conform to your project standards 🙂

@dblanchette dblanchette merged commit c0c9d09 into coveooss:main Nov 29, 2024
5 checks passed
dblanchette pushed a commit that referenced this pull request Nov 29, 2024
## [1.3.1](v1.3.0...v1.3.1) (2024-11-29)

### Bug Fixes

* Fix markdown "break on newline" configurability ([#236](#236)) ([c0c9d09](c0c9d09))
@dblanchette
Copy link
Collaborator

🎉 This PR is included in version 1.3.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants