search

facebook / sapling

A Scalable, User-Friendly Source Control System.
https://sapling-scm.com
GNU General Public License v2.0
6.18k stars 285 forks source link

[github] update _strip_stack_information() to recognize \r\n #863

Closed bolinfest closed 7 months ago

bolinfest commented 7 months ago

[github] update _strip_stack_information() to recognize \r\n

By default, Sapling programmatically writes the PR body on GitHub and will continue to overwrite it every time the user runs sl pr submit. In so doing, Sapling always uses \n as the line terminator for the content it generates.

However, if the user has github.preserve-pull-request-description=true set and edits the pull request body on GitHub, then the next time Sapling pulls down the pull request body to parse it, it will find that the original line terminators it wrote will have been changed to \r\n.

Previous to this commit, _strip_stack_information() always looked for _HORIZONTAL_RULE and _SAPLING_FOOTER_MARKER joined with \n, which failed to match the newly written version containing \r\n. This meant that users with github.preserve-pull-request-description=true set would end up getting an extra copy of the stack information written to the PR body after running sl pr submit because the first instance was considered the source of the PR body that Sapling was not supposed to modify based on github.preserve-pull-request-description=true.

This commit changes _strip_stack_information() to match the pattern using a regex that recognizes both \n and \r\n as the line terminator.

Added a doctest to _strip_stack_information() and verified the following passes locally:

./tests/run-tests.py ./tests/test-doctest.py
facebook-github-bot commented 7 months ago

@facebook-github-bot has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

facebook-github-bot commented 7 months ago

@quark-zju merged this pull request in facebook/sapling@cf8b16fbd2717b77b51de0d296486cd43376863c.