Skip to content
This repository was archived by the owner on Jun 21, 2023. It is now read-only.

Pull Request inline comment appears on GitHub.com but not in extension #1469

Closed
jcansdale opened this issue Feb 8, 2018 · 2 comments
Closed
Labels

Comments

@jcansdale
Copy link
Collaborator

jcansdale commented Feb 8, 2018

  • GitHub Extension for Visual Studio version: 2.4.1
  • Visual Studio version: All

What happened (with steps, logs and screenshots, if possible)

  1. Added inline comment using GitHub Extension: Ensure that the GitHubPaneViewModel is initalized. #1426 (review)
  2. Comment appears on PR page (possibly the line above where I added it, not sure)
  3. Open IPullRequestSessionManager.cs from PR details view
  4. Inline comment doesn't appear on diff view or PR file tree

image

Diagnosis

It appears the issue caused a dependency in the use of the git diff --indent-heuristic option. GitHub.com uses this but command line and libgit2 doesn't use it by default.

You can see the difference in the output from the following commands (on the github/VisualStudio repo):

git diff a39d0bb8..87058c77
git diff a39d0bb8..87058c77 --indent-heuristic

See here for when it was introduced:
https://github.com/github/experience-engineering-code/issues/483

Fix

The --indent-heuristic option is available in libgit:
https://github.com/libgit2/libgit2/blob/da8138b01217824cf211fa491608a7b067cf8e43/include/git2/diff.h#L134

But isn't currently available in libgit2sharp.

https://github.com/libgit2/libgit2sharp/blob/master/LibGit2Sharp/Core/GitDiff.cs#L191
Needs to learn about the new flag.

https://github.com/libgit2/libgit2sharp/blob/3f9b415fa1edfc31ce1ec2b4b3d18441c34adfff/LibGit2Sharp/Diff.cs#L21
Needs to know to set it.

We would need a DiffModifiers.IndentHeuristic bool

If you have a GitDiffOptions then you can turn on bit (1 << 31) of the Flags (which unfortunately we don't have access to). See libgit2/libgit2@7e3faf5

@ethomson said he might have a chance to tackle this on on a long flight tomorrow. It would be very much appreciated if he was able to! 😃

Update: it looks like this has changed to GIT_DIFF_INDENT_HEURISTIC = (1u << 18) in recent versions.
libgit2/libgit2@2d9d246

@jcansdale
Copy link
Collaborator Author

jcansdale commented Dec 14, 2018

Here's a hacked version of LibGit2Sharp that should turn this on by default:
LibGit2Sharp.zip

The selected lined below have been added.

image

image

Even with this patched version I can't add a comment on that line. 😭

image

I'll double check the fix and see if I'm missing something. 😕

You can see the difference by doing:

git diff 8887a8..a39d0b --indent-heuristic -- src/GitHub.Exports.Reactive/Services/IPullRequestSessionManager.cs

image

git diff 8887a8..a39d0b --no-indent-heuristic -- src/GitHub.Exports.Reactive/Services/IPullRequestSessionManager.cs

image

@jcansdale
Copy link
Collaborator Author

A proof of concept fix for this works! 🎈

image

@StanleyGoldman StanleyGoldman changed the title Inline comment appears on PR page but not in GitHub extension Inline comment appears on GitHub.com page but not in extension Apr 11, 2019
@StanleyGoldman StanleyGoldman changed the title Inline comment appears on GitHub.com page but not in extension Inline comment appears on GitHub.com PR page but not in extension Apr 11, 2019
@StanleyGoldman StanleyGoldman changed the title Inline comment appears on GitHub.com PR page but not in extension Pull Request inline comment appears on GitHub.com but not in extension Apr 11, 2019
@github-project-automation github-project-automation bot moved this to Done in BUGS Aug 20, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
Archived in project
Development

No branches or pull requests

1 participant