Skip to content

Conversation

@jingfelix
Copy link
Contributor

current diffcmd_header: ^diff.* (.+) (.+)$
fixed diffcmd_header: ^diff .* (.+) (.+)$

Using the old diffcmd_header may result in lines starting with words like "different" being incorrectly matched as a diff header. Fixing this by adding a blank space.

example: this commit in Linux kernel

image

Signed-off-by: jingfelix <jingfelix@outlook.com>
@jingfelix
Copy link
Contributor Author

@cscorley review request

@cscorley
Copy link
Owner

Nice find, thanks!

Confirmed working with this test

    def test_linux_29e1dfc(self):
        with open(
            "tests/casefiles/linux-29e1dfcd5150097f32f34891c85a50d9ead19df3.patch"
        ) as f:
            text = f.read()

        # testing we get to the diff
        path = "net/bluetooth/6lowpan.c"
        expected = headerobj(
            index_path=None,
            old_path=path,
            old_version="357475cceec61b",
            new_path=path,
            new_version="9a75f9b00b5129",
        )

        results = list(wtp.patch.parse_patch(text))
        self.assertEqual(len(results), 1)
        self.assertEqual(results[0].header, expected)

@cscorley
Copy link
Owner

I am curious to know why I chose this regex, though, thinking twice about it. There must be some alternative commands out there that would normally be considered. But, unfortunately, no tests indicate this and it's been too long to remember exactly.

@cscorley
Copy link
Owner

Thinking the better change may be diffcmd_header = re.compile("^diff( .+)? (.+) (.+)$") which just makes the command section optional instead (e.g., diff lao tzu)

Looking at the CVS diffcmd header, we have the same issue: old_cvs_diffcmd_header = re.compile("^diff.* (.+):(.*) (.+):(.*)$") and I'm wondering if the same "fix" applies there

@cscorley
Copy link
Owner

Went through some old Eclipse patches on Bugzilla and was able to find some interesting cases (e.g. https://bugzillaattachments.eclipsecontent.org/bugs/attachment.cgi?id=1701) but none that show a diffcmd without a space

@cscorley cscorley changed the base branch from main to pr-62 November 16, 2024 16:40
@cscorley cscorley merged commit dadb500 into cscorley:pr-62 Nov 16, 2024
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.

2 participants