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

feat(patch text to hunk bounds): support regex for patch texts #83

Merged
merged 2 commits into from Aug 17, 2020

Conversation

TomKristie
Copy link
Contributor

Get the patch range (start, end) from patch text which gives (start, offset). This range is needed so that from a raw old file and raw new file, we can calculate what lines to suggest edits, and what text file content belong in the lines to suggest.

Given patch text from GitHub which gives output i.e. @@ -132,7 +132,7 @@ module Test @@ -1000,7 +1000,7 @@ module Test

map the patch text to a list of patch ranges by use of regex.

Towards #59 🦕

@TomKristie TomKristie requested review from chingor13 and a team August 17, 2020 16:05
@TomKristie TomKristie requested a review from a team as a code owner August 17, 2020 16:05
@codecov
Copy link

codecov bot commented Aug 17, 2020

Codecov Report

Merging #83 into comment-pr will increase coverage by 1.44%.
The diff coverage is 100.00%.

Impacted file tree graph

@@              Coverage Diff               @@
##           comment-pr      #83      +/-   ##
==============================================
+ Coverage       85.86%   87.30%   +1.44%     
==============================================
  Files              14       15       +1     
  Lines            1238     1379     +141     
  Branches           81       92      +11     
==============================================
+ Hits             1063     1204     +141     
  Misses            174      174              
  Partials            1        1              
Impacted Files Coverage Δ
...ler/comment-handler/github-patch-format-handler.ts 100.00% <100.00%> (ø)
src/types/index.ts 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fd93216...269045d. Read the comment docs.

Comment on lines 30 to 33
const REGEX_MANY_MANY = /@@ -([0-9]+,[0-9]+) \+([0-9]+,[0-9]+) @@\n/g;
const REGEX_ONE_MANY = /@@ -([0-9]+) \+([0-9]+,[0-9]+) @@\n/g;
const REGEX_ONE_ONE = /@@ -([0-9]+) \+([0-9]+) @@\n/g;
const REGEX_MANY_ONE = /@@ -([0-9]+) \+([0-9]+,[0-9]+) @@\n/g;
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's rename these and document what scenarios these cases match to rather than the representation.

const REGEX_MANY_ONE = /@@ -([0-9]+) \+([0-9]+,[0-9]+) @@\n/g;

/**
* Parses the GitHub line-based patch text
Copy link
Contributor

Choose a reason for hiding this comment

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

Parses this into what/for what purpose?

},
];
it('parses original text for multiline modifies', () => {
const patch = gitHubListFilesData[0];
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's split out these fixtures and rename them so it's clear what the scenario is. Reading this test case by itself makes it hard to find the input data.

patch: '@@ -1 +0,0 @@\n-hello world',
},
];
it('parses original text for multiline modifies', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

We have 4 regexes/cases above, but only 3 test cases.

@chingor13 chingor13 merged commit 1531c8b into googleapis:comment-pr Aug 17, 2020
chingor13 added a commit that referenced this pull request Sep 25, 2020
…n pull requests (#105)

* feat(patch text to hunk bounds): support regex for patch texts (#83)

* fix(patch text to hunk bounds): support regex for patch texts

* more comments and more tests

* fix(framework-core): core-library get remote patch ranges (#84)

* fix(framework-core): given files old content and new content, compute the valid hunks (#86)

* fix(framework-core): parse raw changes to ranges

* refactor(framework-core): rename modules, functions, & re-org project structure (#89)

* fix(framework-core): hunk to patch object (#91)

* feat: build failure message from invalid hunks (#90)

* test: add failing stub and test for building the failure message

* fix: implement message building

* fix: use original line numbers in error message

* docs: add docstring

* docs: add note about empty input returning empty string

* feat(framework-core): comment on prs given suggestions (#93)

* feat(framework-core): main interface for create review on a pull request (#114)

* feat(framework-core): main interface for create review on a pull request

* docs: fix typo

* nits and typos...

* gts lint warning fix

* fix(framework-core): combine review comments (#116)

* fix(framework-core): collapsing timeline and inline comments into single review

* test: fixed imports

* added case when there are out of scope suggestions and no valid suggestions

* feat(framework-core): return review number and variable renaming (#117)

* feat(framework-core): return review number and variable renaming

* lint

Co-authored-by: Jeff Ching <chingor@google.com>
Co-authored-by: Justin Beckwith <justin.beckwith@gmail.com>
Co-authored-by: Benjamin E. Coe <bencoe@google.com>
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.

None yet

2 participants