Skip to content

Navigation to diff view from description page#750

Merged
rebornix merged 11 commits intomasterfrom
rebornix/navigate-diffhunk-from-descriptionview
Jan 7, 2019
Merged

Navigation to diff view from description page#750
rebornix merged 11 commits intomasterfrom
rebornix/navigate-diffhunk-from-descriptionview

Conversation

@rebornix
Copy link
Member

@rebornix rebornix 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);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed ;)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice

@rebornix
Copy link
Member Author

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]));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

log something to debug if reveal fails?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

});
}

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@rebornix
Copy link
Member Author

@RMacfarlane comments addressed ;)

@rebornix rebornix merged commit 719cd78 into master Jan 7, 2019
@RMacfarlane RMacfarlane deleted the rebornix/navigate-diffhunk-from-descriptionview branch February 16, 2019 00:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants