browser-extensions
browser-extensions copied to clipboard
fix: code intel on diffs with newly added files
This PR changes:
Fixes https://github.com/sourcegraph/sourcegraph/issues/605
This blue border on newly added files in diffs counted as a row, and since it doesn't have any class names, it isn't filtered out in isCode. Hence, we can't assume that line is non-null.

Testing plan:
I've tested on this PR: https://github.com/sourcegraph/sourcegraph/pull/586/files. The browser extension would break once scrolling past licenseusercount.go. I confirmed this PR fixes the issue so code intel works.
I have tested on:
- [x] Chrome
- [ ] Firefox
- [ ] Safari
- [ ] Phabricator Bundle
I still see a lot of glitchiness with extensions enabled on this PR:
- Sometimes the hover will be duplicated (row of type signature, row of docs, row of type signature, row of docs)
- Sometimes the hover won't show up at all (and there is no error)
I don't think this PR causes the first issue, but it may cause or exacerbate the 2nd one. This PR is likely a partial fix, but we can't be sure when there are so many other issues, so let's fix those first before merging this PR.
I have been testing this with:
- The
Use Sourcegraph extensionsbrowser ext feature flag enabled - and ONLY the https://sourcegraph.com/extensions/sqs/sourcegraph-hello-world extension enabled (which makes it easier to isolate problems in our DOM code vs. the extensions).
Strangely, I can't repro the original issue. All hovers work, above and below the added file.
I followed your conditions above using browser ext version 2.0.0, and I can still repro the original issue.
- Sometimes the hover will be duplicated (row of type signature, row of docs, row of type signature, row of docs)
- Sometimes the hover won't show up at all (and there is no error)
@sqs I can't repro either of these issues with this branch, can you point me to particular reproducible examples?
I don't think this PR causes the first issue, but it may cause or exacerbate the 2nd one. This PR is likely a partial fix, but we can't be sure when there are so many other issues, so let's fix those first before merging this PR.
I don't see how this PR would exacerbate the second issue. It simply checks adds a check to make sure that we are trying to add a hover to a line of code, and not one of the blue borders. I could see my recent commit in sourcegraph-extension-api exacerbating this, though, is that what you're referring to? https://github.com/sourcegraph/sourcegraph-extension-api/commit/f7a6d8541aca55a916b7050e9a0c8ed5b72696e9
Here's how to move this PR to https://github.com/sourcegraph/sourcegraph/tree/master/packages/browser-extensions
cd browser-extensions
git format-patch master --stdout > /tmp/patch
cd ../sourcegraph
cat /tmp/patch | git am -3 --directory=packages/browser-extensions/
# and fixup merge conflicts