Skip to content

Refactor blob and line decoration components

Warren Gifford requested to merge line-decoration-stack-fix into main

Created by: tjkandala

Bugs this refactor aims to fix

  • Fix #15063 (closed). Line decoration attachments weren't cleared properly when they 'stack' from multiple extensions on the same line. To fix this . Also, multiple attachments per line
Line decoration behavior before
Line decoration behavior after
  • Fix #14965 (closed). Extensions used to receive text content from the previous file more often than not.
Hovers for the previously viewed file `fromHoverMerged` on line 19 flatExtensionApi.ts was the previously viewed file
  • Blob used to render once for every line decoration added. This made whole-file decorations for git blame extremely slow on larger files.

Screenshot from 2020-11-02 16-08-53

I added a good amount of tests to hopefully prevents regressions as a result of this refactor, but I may have left some essential functionality uncovered.

Added tests

  • Integration test for line decoration stacking/clearing
  • Integration test for extension text document content
  • Unit test for decoration attachment styling

Misc

The new Blob component has some unusual usage of React hooks, but I've explained in comments why it's necessary to use hoverifier. In the future, we should write a hook to encapsulate these hacks, or better yet, expose an API more conducive to use in function components.

I struggled to pick a name for useSubject. I think it certainly shouldn't share a naming scheme with useObservable, since useSubject returns a Subject and not the value emitted by the Subject, but I'm not sure what the best name to communicate that different would be. Perhaps useRawSubject? Maybe useObservables name should change to useObservableValue?

Merge request reports

Loading