Skip to content

Conversation

mstv
Copy link
Member

@mstv mstv commented Dec 6, 2024

Fixes #12066

Proposed changes

  • PatchProcessor.StartOfContentsRegex: Move '-' to first position inside "[ -+@]" in order to avoid special meaning in regex.
    This made CreatePatchFromString choose the wrong encoding for removed line.

Screenshots

Before

image

After

image

Test methodology

  • manual

Merge strategy

I agree that the maintainer squash merge this PR (if the commit message is clear).


✒️ I contribute this code under The Developer Certificate of Origin.

@mstv mstv self-assigned this Dec 6, 2024
Copy link
Member

@RussKie RussKie left a comment

Choose a reason for hiding this comment

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

Do we have any tests coving this regression?

Copy link
Member

@gerhardol gerhardol left a comment

Choose a reason for hiding this comment

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

Good catch!
A weird range is still a range

mstv and others added 2 commits December 7, 2024 19:20
fix(PatchProcessor): Fixup StartOfContentsRegex

Refs: gitextensions#12091
fix(PatchProcessor): Fixup StartOfContentsRegex
Similar for GetSubmodulesInfo() (in submodules form and  Plugins
for statistics).

Refs: gitextensions#12091
@@ -39,7 +39,7 @@ private enum PatchProcessorState
[GeneratedRegex(@$"(---|\+\+\+) [""]?[^/\s]+/(?<filename>.*)[""]?", RegexOptions.ExplicitCapture)]
private static partial Regex FileNameRegex();

[GeneratedRegex(@$"^({_escapeSequenceRegex})?[ -+@]", RegexOptions.ExplicitCapture)]
[GeneratedRegex(@$"^({_escapeSequenceRegex})?[-+@ ]", RegexOptions.ExplicitCapture)]
Copy link
Member

Choose a reason for hiding this comment

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

To further clarify

Suggested change
[GeneratedRegex(@$"^({_escapeSequenceRegex})?[-+@ ]", RegexOptions.ExplicitCapture)]
[GeneratedRegex(@$"^({_escapeSequenceRegex})?[\-+@ ]", RegexOptions.ExplicitCapture)]

I searched for other occurrences, found one other, with no importance.
Also some testcases.
Pushed to private branch, suggest it is included

@mstv mstv merged commit c1d0a53 into gitextensions:master Dec 8, 2024
4 checks passed
@mstv mstv deleted the fix/12066_encoding branch December 8, 2024 22:26
@mstv mstv added this to the v5.2 milestone Jan 11, 2025
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.

Inconsistent Encoding inside diff
3 participants