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

Ensure changelog render is consistent with all line separator #183

Merged
merged 5 commits into from
Jul 10, 2023

Conversation

danygold
Copy link
Contributor

@danygold danygold commented Jul 9, 2023

Pull Request Details

This pull request should fix #182 , and ensures that changelog render is consistent with all line separator.
In this moment, LF line separator works properly, but instead CRLF and LF have some bugs/different behaviour.

Description

While the purpose of PR #177 was only to fix a regresion, the goal of this PR is to ensures that changelog render is unbiased to line separator.

In particular:

  • Added a new optional parameter lineSeparator to markdownToHTML helper method
  • markdownToHTML and markdownToPlainText now convert line separator to LF before AST parsing.
  • Fixed reformat method (Some regex work only with \n or \r, but not with \r\n, due to missing group)
  • Added a new assertText method, useful for test that plain text output strings are equals (Without improperly using assertMarkdown)
  • Ensure that processOutput normalize line separator also for OutputType.MARKDOWN (In same cases, joinToString and reformat is not enough and generate a content that contains different line separators)

Related Issue

Motivation and Context

While the purpose of PR #177 was only to fix a regresion, the goal of this PR is to ensures that changelog render is unbiased to line separator. See also #182 to see some cases that this PR fix.

The solution applied is the same of #177, so:

  • Before parsing changelog using markdown library, convert line separator to LF.
  • After parsing, restore the correct line separator.
    but this time the behaviour is applied to all code that use the markdown library.

See also markdown bug

How Has This Been Tested

I have added some unit test, that cover my modification. All new and existing tests passed.

Types of changes

  • Docs change / refactoring / dependency upgrade
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • I have read the CONTRIBUTING document.
  • My code follows the code style of this project.
  • My change requires a change to the README.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@hsz
Copy link
Member

hsz commented Jul 10, 2023

Great job, Daniele — thanks!

@hsz hsz added this to the 2.2.2 milestone Jul 10, 2023
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.

Wrong render in changelog file that use CRLF
2 participants