The Wayback Machine - https://web.archive.org/web/20200704192611/https://github.com/github/VisualStudio/issues/1469
Skip to content

/ VisualStudio

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

Jump to bottom

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

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

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

jcansdale opened this issue Feb 8, 2018 · 2 comments
Labels
bug
Projects
BUGS 2.4.4

Comments

@jcansdale
Copy link
Collaborator

@jcansdale jcansdale commented Feb 8, 2018 •
edited

  • 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: #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:
github/c2c-workflows#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 jcansdale added the bug label Feb 8, 2018
@jcansdale jcansdale mentioned this issue Feb 8, 2018
Closed
@meaghanlewis meaghanlewis added this to In Progress in BUGS Feb 8, 2018
@grokys grokys added this to To do in 2.4.3 Feb 8, 2018
@meaghanlewis meaghanlewis added this to To do in 2.4.4 Mar 8, 2018
@meaghanlewis meaghanlewis removed this from To do in 2.4.3 Mar 8, 2018
@meaghanlewis meaghanlewis moved this from In Progress to Needs Repro in BUGS Mar 8, 2018
@grokys grokys mentioned this issue Apr 5, 2018
Closed
8 of 18 tasks complete
@grokys grokys mentioned this issue Jun 14, 2018
Open
@jcansdale
Copy link
Collaborator Author

@jcansdale jcansdale commented Dec 14, 2018 •
edited

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. 😕

  • It looks like Libgit2Sharp 0.23.1 is using LibGit2 0.25.1, which is before the version that includes the GIT_DIFF_INDENT_HEURISTIC option. 😭
  • Libgit2Sharp 0.24.1 is using LibGit2 0.26.2 (which also doesn't contain the option).
  • Targeting .NET 4.6.0 was removed here libgit2/libgit2sharp#1511
  • Recent versions of LibGit2Sharp target .NET 4.6.1 (libgit2/libgit2sharp#1606) which isn't compatible with Visual Studio

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 jcansdale mentioned this issue Dec 18, 2018
Merged
3 of 3 tasks complete
@jcansdale
Copy link
Collaborator Author

@jcansdale jcansdale commented Dec 18, 2018

A proof of concept fix for this works! 🎈

image
@jcansdale jcansdale mentioned this issue Dec 19, 2018
Closed
BUGS automation moved this from Needs Repro to Done Mar 26, 2019
@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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug
Projects
BUGS
  
Done
2.4.4
  
To do
Milestone
No milestone
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
1 participant
@jcansdale
You can’t perform that action at this time.