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

Adopt Github conventions in HTML -> MD conversion #13608

Closed
wants to merge 2 commits into from

Conversation

zm-cttae
Copy link

@zm-cttae zm-cttae commented Mar 4, 2023

Suggested merge commit message (convention)

Closes ckeditor/github-writer#438.
Adopt Github conventions in HTML -> MD conversion.


Additional information

CommonMark is so popular compared to Josh Gruber's original Markdown spec that we should favor it.

@zm-cttae
Copy link
Author

zm-cttae commented Jul 7, 2023

@arkflpc please review this sometime! :)

@arkflpc
Copy link
Contributor

arkflpc commented Jul 10, 2023

As it currently stands, the plugin emits * for unordered lists, which is a valid way to denote unordered lists (CommonMark lists both as valid too). The change to use - instead of * seems to be more of a preference than a requirement or an improvement in functionality.
Could you please clarify what advantages this change would bring?

@zm-cttae
Copy link
Author

zm-cttae commented Jul 10, 2023

The output of the editor would look the same as:

  • the Github help pages,
  • the CommonMark reference's sample code blocks &
  • the vast majority of Markdown files on GitHub

This is also a popular rule in Markdownlint. The status quo is a blocker for some orgs

@arkflpc
Copy link
Contributor

arkflpc commented Jul 11, 2023

Thank you for your response and providing the rationale behind the proposed change.

  1. It would be great if you could create an issue on the repository and describe the change and its rationale there. This will allow for a more comprehensive discussion and provide a central place for tracking the proposed enhancement.
  2. Currently, the pull request lacks test coverage. To contribute to the project, it would be helpful to follow the contributing guidelines, which typically include adding tests to ensure the stability and correctness of the code changes.
  3. While your contribution is appreciated, the suggested change is considered a breaking change (BC) since it alters the default behavior. To address this, it would be beneficial if you could add this change as a configuration option that can be modified, rather than altering the default behavior outright. This approach allows users to choose the rendering style they prefer.

@zm-cttae
Copy link
Author

All good feedback. I will put a formal writeup in the existing issue.

@zm-cttae
Copy link
Author

Writeup is done.

@CKEditorBot
Copy link
Collaborator

There has been no activity on this PR for the past year. We've marked it as stale and will close it in 30 days. We understand it may still be relevant, so if you're interested in the contribution, leave a comment or reaction under this PR.

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

Successfully merging this pull request may close these issues.

[opinionated] Prefer emitting - over * for unordered list markup
3 participants