fix(git): commit bodies with carriage returns are correctly split by …

[MattBelle]

... line

This should enable us to correctly parse squashed commits made on an SCM server residing on a Windows OS.

Description

Checklist

• I have read the contributing guidelines

Code Changes

• Add test cases to all the changes you introduce
• Run uv run poe all locally to ensure this change passes linter check and tests
• Manually test the changes:
  • Verify the feature/bug fix works as expected in real-world scenarios
  • Test edge cases and error conditions
  • Ensure backward compatibility is maintained
  • Document any manual testing steps performed
• Update the documentation for the changes

Documentation Changes

• Run uv run poe doc locally to ensure the documentation pages renders correctly
• Check and fix any broken links (internal or external) in the documentation

When running uv run poe doc, any broken internal documentation links will be reported in the console output like this:

INFO    -  Doc file 'config.md' contains a link 'commands/bump.md#-post_bump_hooks', but the doc 'commands/bump.md' does not contain an anchor '#-post_bump_hooks'.

Expected Behavior

Commit bodies that use carriage returns for line breaks will now be successfully split when getting processed. In particular this enables commitizen to process squash commits made on an SCM server that is hosted by a Windows OS.

Steps to Test This Pull Request

1. Make a dummy squash commit with carriage returns (\r\n):

ci: example ci change

fix: example fix change

feat: example feat change

additional context for example feat

test: example test change

2. Run cz changelog --dry-run and verify that only the feat and fix entries are included
3. Amend the previously made commit to use standard line feeds (\n)
4. Run cz changelog --dry-run and verify that only the feat and fix entries are included

Additional Context

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.88%. Comparing base (d1f8bf0) to head (df198a8).
⚠️ Report is 2 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #1780   +/-   ##
=======================================
  Coverage   97.88%   97.88%           
=======================================
  Files          60       60           
  Lines        2606     2606           
=======================================
  Hits         2551     2551           
  Misses         55       55           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[bearomorphism]

Overall good, but I noticed a small inconsistency between the test and the implementation

[bearomorphism]

qq: is it possible that rev, parents, etc. end with "\r\n"?

If not, could you add comments here about why there will be possibly both CRLF and LF in the `rev_and_commit string?

[MattBelle]

Good catch! While I cannot say definitively, my SCM server only appears to be attaching CRLFs on the body not the metadata. The metadata still appears to be using normal LFs.

[bearomorphism]

I prefer body_newline.join( original lines ov rev_and_commit ).

The inconsistency between this test and the implementation may cause confusion to future contributors.

[bearomorphism]

Or you can add test cases when LF and CRLF are mixed in message.

Also, it would be better to add context on why we are covering those test cases. For example:

# This should enable us to correctly parse squashed commits made on an SCM server residing on a Windows OS.

[MattBelle]

I think my latest commit matches your request? I made the test consistent with the implementation. Let me know if I am wrong and misinterpreted you.

[bearomorphism]

Should we split rev, parents, title, author, author_email with "\n" only?

[MattBelle]

As mentioned in my above comment I think that would technically work. My preference is we stick with my provided implementation (everything uses splitlines())

• it still handles \n
• is easier to read
• splitting the output by line seems to be the intended goal of that line

Furthermore, I think there is minimal risk of it causing a bug since the parts that theoretically don't need the extra delimiter logic are all computer generated. That said, I'd be happy to piece out the logic and only apply the splitlines() call to the body if that's what you want.

[bearomorphism]

LGTM, thanks!
@woile @Lee-W @noirbizarre could you also take a look?