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

Mark/unmark identical files as “Viewed” in the PR files tab #3045

Open
mfulton26 opened this issue May 1, 2020 · 8 comments
Open

Mark/unmark identical files as “Viewed” in the PR files tab #3045

mfulton26 opened this issue May 1, 2020 · 8 comments

Comments

@mfulton26
Copy link

mfulton26 commented May 1, 2020

Periodically I end up reviewing large PRs that have lots of files that have similar identical diffs (e.g. package rename causes imports and dependencies in package.json files to change, etc.).

I realized it would be great to have a way to mark similar identical files as “Viewed” in the PR files tab.

A neat way to invoke this could be by double-clicking a “Viewed” checkbox.

I ended up doing this through a custom search engine in Chrome for now with the below code as its "URL" (I have its keyword defined as gh+~v so that on my Mac all I have to do is mark a few files as “Viewed” and then type Command+L+gh+#v+Enter and then all similar identical diffs get marked as “Viewed” too; I have similar shortcuts like gh+v to mark all files as “Viewed”, ghrd to display rich diffs for all files, etc.).

javascript: markIdenticalFilesAsViewed();

async function markIdenticalFilesAsViewed() {
  const files = document.querySelectorAll(".file");
  const { markedFiles, unmarkedFiles } = groupFiles(files);
  const markedHashes = new Set(markedFiles.map(getFileDiffHash));
  const identicalFiles = unmarkedFiles.filter((file) =>
    markedHashes.has(getFileDiffHash(file))
  );
  const { length: found } = identicalFiles;
  if (found === 0) {
    alert(`All identical files are marked as “Viewed”.`);
  } else if (confirm(`Mark ${found} identical files as “Viewed”?`)) {
    const start = Date.now();
    for await (const file of periodicEventCycling(identicalFiles)) {
      file.querySelector(".js-reviewed-checkbox").click();
    }
    const end = Date.now();
    const elapsed = ((end - start) / 1000).toLocaleString();
    alert(`Marked ${found} identical files as “Viewed” in ${elapsed} seconds.`);
  }
}

function groupFiles(files) {
  const markedFiles = [];
  const unmarkedFiles = [];
  for (const file of files) {
    const group = file.querySelector(".js-reviewed-checkbox[checked]")
      ? markedFiles
      : unmarkedFiles;
    group.push(file);
  }
  return { markedFiles, unmarkedFiles };
}

function getFileDiffHash(file) {
  const deletions = Array.from(
    file.querySelectorAll(".blob-code-deletion .blob-code-inner"),
    (e) => e.innerText
  );
  const additions = Array.from(
    file.querySelectorAll(".blob-code-addition .blob-code-inner"),
    (e) => e.innerText
  );
  return JSON.stringify({ deletions, additions });
}

async function* periodicEventCycling(iterable, delay = 50) {
  let updated = Date.now();
  for (const value of iterable) {
    yield value;
    if (Date.now() - updated > delay) {
      await new Promise((resolve) => setTimeout(resolve, 0));
      updated = Date.now();
    }
  }
}
@sindresorhus
Copy link
Member

Please define “similar files”. What’s the heuristics?

@fregante
Copy link
Member

fregante commented May 1, 2020

“Similar” is hard; your code is for “identical diffs,” which is more reasonable.

Just recently I pushed here a few “search/replace PRs” that would have benefited from such a feature, however some of them only had “similar” diffs.

The issues I see for this are UI and performance.

If the PR has many files (which is what this feature would be for), reading and comparing the diffs might not be instant. It should skip files with large diffs, which are unlikely to be identical anyway.

As for the UI, how would you trigger this behavior? Maybe any simple click on “Viewed” should trigger a confirm() dialog:

There are 4 more files with identical diffs, do you also want to mark them as Viewed?

@fregante
Copy link
Member

fregante commented May 1, 2020

I’ll add: I don’t think “similar” is advisable at all. Even a single character might be significant.

@mfulton26
Copy link
Author

Yes, you are right, "identical"/"same" would be a better word for it. I like the idea of a confirm window. Another idea might be to put a button "Mark this and 14 identical diffs" or something but that requires UI. Thank you for exploring this with me to see if we can come up with a good application for this and potentially make it an awesome feature.

@mfulton26 mfulton26 changed the title Mark/unmark similar files as “Viewed” in the PR files tab Mark/unmark identical files as “Viewed” in the PR files tab May 1, 2020
@mfulton26
Copy link
Author

FYI: I've updated my code snippet in the description to use setTimeout instead of requestAnimationFrame as the latter is paused while the tab isn't "visible" which can suspend the entire script until the user returns to the tab, etc.

@fregante
Copy link
Member

fregante commented May 6, 2020

PR welcome, but it has to start from a clicked file, it shouldn't just mark "any identical diff" like the snippet in the first comment suggests.

Something simple like:

on "Viewed" click (if it's not triggered by `batch-mark-files-as-viewed`)
	look for identical files
	ask confirmation
	click each identical file 

Notes:

  • no need for the suggested periodicEventCycling function

@mfulton26
Copy link
Author

no need for the suggested periodicEventCycling function

I originally had to do that to prevent blocking the UI from updating on large PRs but it may not necessary (especially as it seems that GitHub has improved some things related to bulk marking as “Viewed” lately)

@mfulton26
Copy link
Author

no need for the suggested periodicEventCycling function

I have some local code I'm testing out in this extension's codebase, for large PRs there is no visual progress updating without something like periodicEventCycling and so the UI freezes until all the clicks have taken place (which takes a bit)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

3 participants