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

Fix a couple of CommentAsPatch issues. #14804

Merged
merged 5 commits into from
Feb 27, 2021

Conversation

zeripath
Copy link
Contributor

@zeripath zeripath commented Feb 25, 2021

CutDiffAroundLine makes the incorrect assumption that --- and +++ always represent part of the header of a diff.

This PR adds a flag to its parsing to prevent this problem and adds a streaming parsing technique to CutDiffAroundLine using an io.pipe instead of just sending data to an unbounded buffer.

Handle unquoted comment patch filenames

When making comment patches unfortunately the patch does not always quote the filename. This makes the diff --git header ambiguous again.

This PR finally adds handling for ambiguity in to parse patch

Fix #14711
Fix #14812

Signed-off-by: Andrew Thornton [email protected]


@zeripath
Copy link
Contributor Author

Cutdiffaroundline also needs to have some protection against truly pathological diffs. E.g. single line minimised js file becomes unminimised and you comment on one of the lines

It's also essentially a duplicating some of the parsepatch code.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Feb 25, 2021
modules/git/diff_test.go Outdated Show resolved Hide resolved
@zeripath zeripath changed the title Fix incorrect patch in comments on or around lines containing initial double dash or plus Fix a couple of CommentAsPatch issues. Feb 27, 2021
… always represent part of the header of a diff.

This PR adds a flag to its parsing to prevent this problem and adds a streaming parsing technique to CutDiffAroundLine using an io.pipe instead of just sending data to an unbounded buffer.

Fix go-gitea#14711

Signed-off-by: Andrew Thornton <[email protected]>
When making comment patches unfortunately the patch does not always quote the filename
This makes the diff --git header ambiguous again.

This PR finally adds handling for ambiguity in to parse patch

Fix go-gitea#14812

Signed-off-by: Andrew Thornton <[email protected]>
There is no way currently for CutDiffAroundLine in this test to cause an
error however, it should still be tested.

Signed-off-by: Andrew Thornton <[email protected]>
modules/git/diff.go Outdated Show resolved Hide resolved
@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Feb 27, 2021
Signed-off-by: Andrew Thornton <[email protected]>
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Feb 27, 2021
@6543
Copy link
Member

6543 commented Feb 27, 2021

🚀

@6543 6543 merged commit 3d8b5ad into go-gitea:master Feb 27, 2021
@zeripath zeripath deleted the fix-oracle-diff-issue branch February 27, 2021 19:00
zeripath added a commit to zeripath/gitea that referenced this pull request Feb 27, 2021
Backport go-gitea#14804

* CutDiffAroundLine makes the incorrect assumption that `---` and `+++` always represent part of the header of a diff.

This PR adds a flag to its parsing to prevent this problem and adds a streaming parsing technique to CutDiffAroundLine using an io.pipe instead of just sending data to an unbounded buffer.

Fix go-gitea#14711

Signed-off-by: Andrew Thornton <[email protected]>

* Handle unquoted comment patch files

When making comment patches unfortunately the patch does not always quote the filename
This makes the diff --git header ambiguous again.

This PR finally adds handling for ambiguity in to parse patch

Fix go-gitea#14812

Signed-off-by: Andrew Thornton <[email protected]>

* Add in testing for no error

There is no way currently for CutDiffAroundLine in this test to cause an
error however, it should still be tested.

Signed-off-by: Andrew Thornton <[email protected]>
lafriks pushed a commit that referenced this pull request Feb 28, 2021
Backport #14804

* CutDiffAroundLine makes the incorrect assumption that `---` and `+++` always represent part of the header of a diff.

This PR adds a flag to its parsing to prevent this problem and adds a streaming parsing technique to CutDiffAroundLine using an io.pipe instead of just sending data to an unbounded buffer.

Fix #14711

* Handle unquoted comment patch files

When making comment patches unfortunately the patch does not always quote the filename
This makes the diff --git header ambiguous again.

This PR finally adds handling for ambiguity in to parse patch

Fix #14812

* Add in testing for no error

There is no way currently for CutDiffAroundLine in this test to cause an
error however, it should still be tested.

Signed-off-by: Andrew Thornton <[email protected]>
@zeripath zeripath added the backport/done All backports for this PR have been created label Mar 1, 2021
@go-gitea go-gitea locked and limited conversation to collaborators May 13, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
backport/done All backports for this PR have been created lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/bug
Projects
None yet
4 participants