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

conflicts: whitespace parsing fixes #4944

Merged
merged 4 commits into from
Nov 23, 2024

Conversation

scott2000
Copy link
Contributor

I noticed these while working on #4897 and I thought I'd make a PR for them.

Checklist

If applicable:

  • I have updated CHANGELOG.md
  • I have updated the documentation (README.md, docs/, demos/)
  • I have updated the config schema (cli/src/config-schema.json)
  • I have added tests to cover my changes

lib/tests/test_conflicts.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@ilyagr ilyagr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks fine to me. I'll wait a bit before approving in case there are other comments.

lib/src/conflicts.rs Outdated Show resolved Hide resolved
Currently, conflict markers ending in CRLF line endings aren't allowed.
I don't see any reason why we should reject them, since some
editors/tools might produce CRLF automatically on Windows when saving
files, which would break the conflicts otherwise.
Some editors strip trailing whitespace on save, which breaks any diffs
which have context lines, since the parsing function expects them to
start with a space. There's no visual difference between " \n" and "\n",
so it seems reasonable to accept both.
@scott2000 scott2000 force-pushed the conflict-whitespace-fixes branch from 7f2da27 to 196fbef Compare November 22, 2024 23:20
@scott2000 scott2000 merged commit 6e959fa into jj-vcs:main Nov 23, 2024
18 checks passed
@scott2000 scott2000 deleted the conflict-whitespace-fixes branch November 23, 2024 00:00
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.

3 participants