3

I have a webhook that listen to all code reviews, then I fetch the comments of this PR review in order to get the position of the comment in the diff. I'm using the GitHub REST API, but the issue I have is the same with the GraphQL API.

So the workflow is:

  1. Get the review ID from the webhook
  2. Fetch the comment list of that review
  3. For each comment, get the diff hunk and the position to find the edited line

All of that works fine 99% of the time. Sometimes I get null in the position, I ignore these comments.

But this time, I get another weird issue. Usually, the position refers to the index of the line in the diff.

For example, in:

@@ -1 +1,3 @@
-# sedy-test
\\ No newline at end of file
+# sedy-test
+
+This repository is used to test [sedy](https://github.com/marmelab/sedy).

If the position is 3, the edited line is +# sedy-test.

The issue is that for some comments, I get a position that can't be inside the diff. As an example, see this PR.

When I try to fetch the comment position of the review with the following request:

{
  repository(owner: "Kmaschta", name: "comfygure") {
    pullRequest(number: 1) {
      reviews(last: 1) {
        edges {
          node {
            state
            comments(first: 1) {
              edges {
                node {
                  bodyText
                  authorAssociation
                  position
                  originalPosition
                  diffHunk
                }
              }
            }
          }
        }
      }
    }
  }
}

The response is the following:

{
  "data": {
    "repository": {
      "pullRequest": {
        "reviews": {
          "edges": [
            {
              "node": {
                "state": "COMMENTED",
                "comments": {
                  "edges": [
                    {
                      "node": {
                        "bodyText": "s/fot/for/",
                        "authorAssociation": "OWNER",
                        "position": 71,
                        "originalPosition": 71,
                        "diffHunk": "@@ -24,31 +34,39 @@ const ls = (ui, modules) => function* () {\n };\n \n const add = (ui, modules, options) => function* () {\n-    const { red, bold } = ui.colors;\n+    const { red, bold, green } = ui.colors;\n \n     if (!options.length) {\n         ui.error(`${red('No environment specified.')}`);\n-        help(ui, 1);\n     }\n \n     if (options.length > 1) {\n         ui.error(`${red('Invalid environment format. The environment name should be one word.')}`);\n-        help(ui, 1);\n+    }\n+\n+    if (options.length !== 1) {\n+        ui.print(`${bold('SYNOPSIS')}\n+        ${bold('comfy')} env add <environment>\n+\n+Type ${green('comfy env --help')} for details`);\n+\n+        return ui.exit(0);\n     }\n \n     const project = yield modules.project.retrieveFromConfig();\n     const environment = yield modules.environment.add(project, options[0]);\n-    const addCommand = `comfy add ${environment.name}`;\n+    const addCommand = `comfy setall ${environment.name}`;\n \n-    ui.print(`${bold('Cool!')} Your new environment \"${bold(environment.name)}\" was successfully saved.`);\n-    ui.print(`You can now add a configuration, try ${bold(addCommand)}`);\n+    ui.print(`${bold(green('Environment successfully created'))}`);\n+    ui.print(`You can now set a configuration fot this environment using ${bold(addCommand)}`);"
                      }
                    }
                  ]
                }
              }
            }
          ]
        }
      }
    }
  }
}

The position is 71, but the diff doesn't contain more than 40 lines.

So is it a bug if the GitHub API, or I didn't understand the point of the position field?

Note: I posted the same question on the GitHub forum.

Nimantha
  • 6,405
  • 6
  • 28
  • 69
Kmaschta
  • 2,369
  • 1
  • 18
  • 36

2 Answers2

3

From Github API comment doc :

The position value equals the number of lines down from the first "@@" hunk header in the file you want to add a comment. The line just below the "@@" line is position 1, the next line is position 2, and so on. The position in the diff continues to increase through lines of whitespace and additional hunks until the beginning of a new file.

Here diffHunk gives you the current diff hunk which is not necessary the first in the file

If you get the full diff file it's more clear :

curl "https://api.github.com/repos/Kmaschta/comfygure/pulls/1" \
     -H "Accept: application/vnd.github.v3.diff"

The comment is in env.js whose first hunk starts at line 77, your comment is in line 148 while the diffHunk in your request starts at line 114

I don't think it's possible to request the full PR diff using GraphQL at the moment but you can use Rest v3 as above

Bertrand Martel
  • 42,756
  • 16
  • 135
  • 159
  • Hi, thanks for the answer! How can I know from the diffHunk (or the comment) that the hunk starts at the line 77? Moreover, in most of the case, the first line *in the diff hunk* is 1. I don't get it. – Kmaschta Oct 29 '18 at 17:01
  • 1
    You can't tell from the diffHunk, you need the full diff to know at which line the first hunk of the file starts – Bertrand Martel Oct 29 '18 at 17:05
  • 1
    This works most of the time because most of the time the first hunk for the file is the only one or it's the first of the file. It fails when you have multiple diff in the file – Bertrand Martel Oct 29 '18 at 17:07
1

I have same problem. And I finally find out how to determine the position.

Let's see your PR.

https://github.com/Kmaschta/comfygure/pull/1/files?utf8=%E2%9C%93&diff=unified#diff-10b371776dce3b12ed817f3fb8704a7d

This file has 2 diff-hunks.

The position is start from first hunk.

The line below the first hunk is position 1.

https://github.com/Kmaschta/comfygure/pull/1/files?utf8=%E2%9C%93&diff=unified#diff-10b371776dce3b12ed817f3fb8704a7dL1

And end of the first hunk is position 36.

https://github.com/Kmaschta/comfygure/pull/1/files?utf8=%E2%9C%93&diff=unified#diff-10b371776dce3b12ed817f3fb8704a7dL18

And somehow github add +1 for end of first hunk to before second hunk start.(36+1)

So, the start line of second hunk is 38.

And you have 34 lines above of your comment in second hunk.

That's why your comment is 71.

https://github.com/Kmaschta/comfygure/pull/1/files?utf8=%E2%9C%93&diff=unified#diff-10b371776dce3b12ed817f3fb8704a7dR61

Github's calculation method is this way.

I think this calculation is something wrong. But if you want to calculate, you can do by this method.

ZENN
  • 71
  • 1
  • 5
  • Thanks for the answer! Are you sure this is the correct calculation? I have hard time verifying this. – Kmaschta Nov 06 '18 at 18:15
  • I didn't try all cases but almost all case this method is correct.I also have hard time to find out this. If you find out exceptional case please tell me. – ZENN Nov 07 '18 at 01:34