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

Navigation to diff view from description page #750

Merged
merged 11 commits into from Jan 7, 2019

Conversation

Projects
None yet
2 participants
@rebornix
Copy link
Member

commented Dec 11, 2018

nagivation-diff-view-from-description-page

@rebornix rebornix changed the title Add support for review mode Navigation to diff view from description page Dec 11, 2018

context.subscriptions.push(vscode.commands.registerCommand('pr.openDescription', async (pr: IPullRequestModel) => {
const pullRequest = ensurePR(prManager, pr);
context.subscriptions.push(vscode.commands.registerCommand('pr.openDescription', async (descriptionNode: DescriptionNode) => {
const pullRequest = ensurePR(prManager, descriptionNode.pullRequestModel);

This comment has been minimized.

Copy link
@RMacfarlane

RMacfarlane Dec 12, 2018

Contributor

when opening the description from the command palette or the status bar, descriptionNode can be undefined

This comment has been minimized.

Copy link
@rebornix

rebornix Dec 12, 2018

Author Member

fixed ;)

This comment has been minimized.

Copy link
@RMacfarlane

RMacfarlane Dec 13, 2018

Contributor

nice

@rebornix

This comment has been minimized.

Copy link
Member Author

commented Dec 12, 2018

Added outdated label to comment reviews and we don't support code navigation if the comment is outdated.

image

image

const diffPath: HTMLSpanElement = document.createElement('span');
diffPath.className = outdated ? 'diffPath outdated' : 'diffPath';
diffPath.textContent = comments[0].path;
diffPath.addEventListener('click', () => this.openDiff(comments[0]));

This comment has been minimized.

Copy link
@RMacfarlane

RMacfarlane Dec 13, 2018

Contributor

should the event listener only be added if the comment isn't outdated?

This comment has been minimized.

Copy link
@rebornix

rebornix Dec 18, 2018

Author Member

done

if ((prContainer as TreeNode | Revealable<TreeNode>).revealComment) {
(prContainer as TreeNode | Revealable<TreeNode>).revealComment(comment);
}
} catch (e) {

This comment has been minimized.

Copy link
@RMacfarlane

RMacfarlane Dec 13, 2018

Contributor

log something to debug if reveal fails?

This comment has been minimized.

Copy link
@rebornix

rebornix Dec 18, 2018

Author Member

done

@@ -591,11 +591,24 @@ class ReviewNode {
});
}

let outdated = comments[0].position !== comments[0].original_position;

This comment has been minimized.

Copy link
@RMacfarlane

RMacfarlane Dec 13, 2018

Contributor

in other places we check if position is null to determine if a comment is outdated. I think we should do that here too

This comment has been minimized.

Copy link
@rebornix

rebornix Dec 18, 2018

Author Member

done

@rebornix

This comment has been minimized.

Copy link
Member Author

commented Dec 18, 2018

@RMacfarlane comments addressed ;)

rebornix added some commits Jan 5, 2019

@rebornix rebornix merged commit 719cd78 into master Jan 7, 2019

1 check passed

license/cla All CLA requirements met.
Details

@RMacfarlane RMacfarlane deleted the rebornix/navigate-diffhunk-from-descriptionview branch Feb 16, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.