Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upGitHub is where the world builds software
Millions of developers and companies build, ship, and maintain their software on GitHub — the largest and most advanced development platform in the world.
Navigate to correct repository for inline comment GitHub link where fork repository is open. #1420
Conversation
We sometimes want to get the URL of a parent or fork repository. Add an optional `owner` parameter to `UriString.ToRepositoryUrl` which will change the owner of the produced repository URL.
| var nameWithOwner = owner == null || NameWithOwner == null ? | ||
| NameWithOwner : | ||
| string.Format(CultureInfo.InvariantCulture, "{0}/{1}", owner, RepositoryName); | ||
|
|
shana
Jan 29, 2018
Collaborator
Nit: Given owner = null and NameWithOwner != null, won't this mean that nameWithOwner will be /bar instead of bar? I know it probably doesn't make a difference if the Path in UriBuilderstarts with a/`, it will probably get ignored, but wouldn't this be more straightforward as:
var nameWithOwner = owner != null ? string.Format(CultureInfo.InvariantCulture, "{0}/{1}", owner, RepositoryName) : NameWithOwner;
Also, if RepositoryName is null (it can be), should this probably return null even if owner is passed in, for consistency?
Nit: Given owner = null and NameWithOwner != null, won't this mean that nameWithOwner will be /bar instead of bar? I know it probably doesn't make a difference if the Path in UriBuilderstarts with a/`, it will probably get ignored, but wouldn't this be more straightforward as:
var nameWithOwner = owner != null ? string.Format(CultureInfo.InvariantCulture, "{0}/{1}", owner, RepositoryName) : NameWithOwner;
Also, if RepositoryName is null (it can be), should this probably return null even if owner is passed in, for consistency?
|
@shana I've updated the code with a few more tests and a fix.
Did you mean the parameter
|
|
I've tested this against the original repro and it works great. |
When a fork repository was open in VS, but the PRs for the parent repository were being viewed, clicking on the "date" link in an inline comment to go to the comment online was navigating to the fork repository instead of the parent repository.
To fix this, use the correct repository owner for GitHub link in
InlineCommentThreadViewModel.GetCommentUrl. An optionalownerparameter was added toUriString.ToRepositoryUrl. which will change the owner of the produced repository URL.Fixes #1413