Skip to content

Commit

Permalink
Merge pull request #85 from Alokit-Innovations/akg/better_naming_in_h…
Browse files Browse the repository at this point in the history
…ighlighting

refactored the github hunk highlighting
  • Loading branch information
avikalpg authored Nov 29, 2023
2 parents d9814d1 + d594d58 commit aede68c
Show file tree
Hide file tree
Showing 3 changed files with 83 additions and 68 deletions.
2 changes: 1 addition & 1 deletion manifest.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "Vibinex Code Review",
"version": "1.0.5",
"version": "1.0.6",
"manifest_version": 3,
"description": "Personalization and context for pull requests on GitHub & Bitbucket",
"key": "MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAsRm6EaBdHDBxVjt9o9WKeL9EDdz1X+knDAU5uoZaRsXTmWjslhJN9DhSd7/Ys4aJOSN+s+5/HnIHcKV63P4GYaUM5FhETHEWORHlwIgjcV/1h6wD6bNbvXi06gtiygE+yMrCzzD93/Z+41XrwMElYiW2U5owNpat2Yfq4p9FDX1uBJUKsRIMp6LbRQla4vAzH/HMUtHWmeuUsmPVzcq1b6uB1QmuJqIQ1GrntIHw3UBWUlqRZ5OtxI1DCP3knglvqz26WT5Pc4GBDNlcI9+3F0vhwqwHqrdyjZpIKZ7iaQzcrovOqUKuXs1J3hDtXq8WoJELIqfIisY7rhAvq6b8jQIDAQAB",
Expand Down
145 changes: 80 additions & 65 deletions scripts/highlighting.js
Original file line number Diff line number Diff line change
Expand Up @@ -183,80 +183,95 @@ async function FilesInPrBitbucket(response) {
* @param {Array} apiResponses - Array of API responses containing hunk information.
*/
const githubHunkHighlight = async (apiResponses) => {
// checking for diff view either unified or split
let isUnifiedDiffView = false;
const allInputTagsWithValueAttr = document.querySelectorAll('input[value]'); // for reducing DOM queries in foreach loop.
for (const item of allInputTagsWithValueAttr) {
const valueAttrOfInputTag = item.getAttribute('value');
const checkedAttrOfInputTag = item.getAttribute('checked');

if (valueAttrOfInputTag === 'unified' || valueAttrOfInputTag === 'split') {
if (checkedAttrOfInputTag === 'checked' && valueAttrOfInputTag === 'unified') {
isUnifiedDiffView = true; // for unified view
}
}
};

// TODO: optimization needed to not highlight next deleted line space if not present in response.
const getFileName = Array.from(document.querySelectorAll('div[data-tagsearch-path]'));
getFileName.forEach(async (item) => {
let fileContent = item.getAttribute('data-tagsearch-path');
if (fileContent) {
const allFileDiffViews = Array.from(document.querySelectorAll('div[data-tagsearch-path]'));
allFileDiffViews.forEach(async (fileDiffView) => {
let filepathFromHTML = fileDiffView.getAttribute('data-tagsearch-path');
if (!filepathFromHTML) {
console.error(`[vibinex] File address not found in HTML for file: ${filepathFromHTML}`);
return;
}

const matchEncrypted = await sha256(fileContent);
const foundFiles = apiResponses["hunkinfo"].filter(item => item.filepath === matchEncrypted);
const encryptedFilepathFromHTML = await sha256(filepathFromHTML);
const relevantHunksInThisFile = apiResponses["hunkinfo"].filter(hunkInfo => hunkInfo.filepath === encryptedFilepathFromHTML);

if (foundFiles.length > 0) {
// checking for diff view either unified or split
// TODO: We can identify the view once for all files at once instead of doing it for each file separately
const deletedLines = document.querySelectorAll('input[value]');
let diffView = false;
deletedLines.forEach((item) => {
const getValue = item.getAttribute('value');
const getName = item.getAttribute('checked');
if (relevantHunksInThisFile.length <= 0) {
console.debug(`[vibinex] No relevant hunks in file: ${filepathFromHTML}`);
return;
}

if (getValue == 'unified' || getValue == 'split') {
if (getName == 'checked' && getValue == 'unified') {
diffView = true; // for unified view
}
/** Select all TRs except the following:
* - TRs with the .js-skip-tagsearch are the rows which are expandable (we click on it to see more code)
* - TRs inside the THEAD[hidden] tags only contain the headings of the diff table
*/
const allRowsInFileDiff = Array.from(fileDiffView.querySelectorAll('tr:not(.js-skip-tagsearch):not(thead[hidden] :is(tr))'));
if (isUnifiedDiffView) { // for unified view
const isRelevantRow = (row) => {
const lineContentSpan = row.querySelector('span[data-code-marker]');
if (!lineContentSpan) {
console.warn(`[vibinex] Could not detect span with data-code-marker in diff row for file: ${filepathFromHTML}`);
return false;
}
const signature = lineContentSpan.getAttribute('data-code-marker');

const cellInRowWithLineNumber = row.querySelector('td[data-line-number]');
if (!cellInRowWithLineNumber) {
console.warn(`[vibinex] Could not detect cell with data-line-number in diff row for file: ${filepathFromHTML}`);
return false;
}
const lineNumber = cellInRowWithLineNumber.getAttribute('data-line-number');
for (const hunk of relevantHunksInThisFile) {
if ((signature === '-') && (parseInt(lineNumber) >= parseInt(hunk.line_start)) && (parseInt(lineNumber) <= parseInt(hunk.line_end))) {
return true;
}
}
return false;
};
allRowsInFileDiff.forEach((rowInFileDiff) => {
if (isRelevantRow(rowInFileDiff)) {
rowInFileDiff.style.backgroundColor = GH_RELEVANT_BG_COLOR;
}
});
} else { // for split view
allRowsInFileDiff.forEach((rowInFileDiff) => {
if (rowInFileDiff.classList.contains('blob-expanded')) {
return; // this row is not considered part of the diff
}
const leftSideContentCell = rowInFileDiff.querySelector("td[data-split-side='left']");
if (!leftSideContentCell) {
console.warn(`[vibinex] Could not detect left side content cell in diff row`);
return; // this row doesn't have left side content cell
}
const addCommentButtonWithLineNo = leftSideContentCell.querySelector('button[data-line]');
if (!addCommentButtonWithLineNo) {
console.error('[vibinex] Could not detect line number in diff row');
return; // this cell doesn't have 'add comment' button
}
const lineNumber = addCommentButtonWithLineNo.getAttribute('data-line');

});

const value = Array.from(item.getElementsByTagName('tr'));

if (diffView) {
// for unified view
let flag = false;
value.forEach((item, index) => {
const deletedLines = item.querySelector('button[data-original-line]');
if (deletedLines !== null) {
const originalLine = deletedLines.getAttribute('data-original-line');
const signature = originalLine.charAt(0);
const tableNumber = item.querySelector('td[data-line-number]');
const checkNumber = tableNumber.getAttribute('data-line-number');
for (const foundFile of foundFiles) {
if ((signature == '-' || signature == '+') && parseInt(checkNumber) >= parseInt(foundFile.line_start) && parseInt(checkNumber) <= parseInt(foundFile.line_end)) {
flag = true;
} else {
flag = false;
}

if (flag) {
item.style.backgroundColor = GH_RELEVANT_BG_COLOR;
}
}
if (leftSideContentCell?.querySelector("span[data-code-marker='-']")) {
for (const hunk of relevantHunksInThisFile) {
if (parseInt(lineNumber) >= parseInt(hunk.line_start) && parseInt(lineNumber) <= parseInt(hunk.line_end)) {
leftSideContentCell.style.backgroundColor = GH_RELEVANT_BG_COLOR;
}
});

} else {
// for split view
value.forEach((items) => {
const secondRow = Array.from(items.getElementsByTagName('td'));
secondRow.forEach((item) => {
const buttonId = item.querySelector('button[data-line]');
if (buttonId) {
const dataLineValue = buttonId.getAttribute('data-line');
const tableContent = items.querySelector("td[data-split-side='left']");
if ((tableContent.innerHTML === '') || (tableContent && tableContent.querySelector("span[data-code-marker='-']"))) {
for (const foundFile of foundFiles) {
if (parseInt(dataLineValue) >= parseInt(foundFile.line_start) && parseInt(dataLineValue) <= parseInt(foundFile.line_end)) {
items.style.backgroundColor = GH_RELEVANT_BG_COLOR;
}
}
}
}
});
});
}
}
}
// we currently do not handle any other cases
});
}
});
};
Expand Down
4 changes: 2 additions & 2 deletions scripts/orchestrator.js
Original file line number Diff line number Diff line change
Expand Up @@ -81,9 +81,9 @@ const orchestrator = async (tabUrl, websiteUrl, userId) => {
// for showing all tracked repo in user page
else if (urlObj[3] && !urlObj[4] && searchParams.includes('tab=repositories')) {
// for working on this url https://github.com/username?tab=repositories
const userName = urlObj[3]
const userName = urlObj[3];
const trackedRepos = await getTrackedRepos(userName, userId, 'github');
updateTrackedReposInGitHub(trackedRepos, websiteUrl, 'user')
updateTrackedReposInGitHub(trackedRepos, websiteUrl, 'user');
}
}

Expand Down

0 comments on commit aede68c

Please sign in to comment.