Skip to content
This repository has been archived by the owner on Nov 25, 2021. It is now read-only.

feat: add code view-specific tokenization settings #401

Merged
merged 1 commit into from
Sep 17, 2021

Conversation

tjkandala
Copy link
Contributor

Some code hosts use many types of code views, some of which are tokenized and some of which are not. This PR introduces a way for consumers to override the Hoverifier instance tokenize flag through event options in Hoverifer#hoverify.

@vovakulikov
Copy link

@tjkandala Will you have a follow-up PR for this one? I mean add overrideTokenize at consumer's code-base?

@@ -549,14 +558,18 @@ export function createHoverifier<C extends object, D, A>({
// It's important to do this before filtering otherwise navigating from
// a position, to a line-only position, back to the first position would get ignored
distinctUntilChanged((a, b) => isEqual(a, b)),
map(({ position, codeView, dom, ...rest }) => {
map(({ position, codeView, dom, overrideTokenize, ...rest }) => {
Copy link

Choose a reason for hiding this comment

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

Do you think, can we use a parameter default value assignment, like overrideTokenize = tokenize, ...rest}) instead of shouldTokenize function?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a great idea, but will we always use destructuring assignment? I think keeping it explicit will prevent regressions

@tjkandala
Copy link
Contributor Author

@vovakulikov We'll use this in sourcegraph/sourcegraph#25020!

@tjkandala tjkandala force-pushed the tjk/override-tokenize branch from 896ee30 to 71d58d1 Compare September 17, 2021 20:40
@codecov
Copy link

codecov bot commented Sep 17, 2021

Codecov Report

Merging #401 (71d58d1) into master (6adb829) will increase coverage by 0.04%.
The diff coverage is 90.47%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #401      +/-   ##
==========================================
+ Coverage   85.86%   85.91%   +0.04%     
==========================================
  Files          14       14              
  Lines         651      653       +2     
  Branches      174      175       +1     
==========================================
+ Hits          559      561       +2     
  Misses         92       92              
Impacted Files Coverage Δ
src/hoverifier.ts 85.65% <89.47%> (ø)
src/token_position.ts 91.89% <100.00%> (+0.08%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6adb829...71d58d1. Read the comment docs.

@tjkandala tjkandala merged commit 976a71f into master Sep 17, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants