Skip to content
This repository was archived by the owner on Nov 25, 2021. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 13 additions & 5 deletions src/hoverifier.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,11 @@ export interface HoverifierOptions<C extends object, D, A> {
* Whether or not hover tooltips can be pinned.
*/
pinningEnabled: boolean

/**
* Whether or not code views need to be tokenized. Defaults to true.
*/
tokenize?: boolean
}

/**
Expand Down Expand Up @@ -349,6 +354,7 @@ export function createHoverifier<C extends object, D, A>({
getHover,
getActions,
pinningEnabled,
tokenize = true,
}: HoverifierOptions<C, D, A>): Hoverifier<C, D, A> {
// Internal state that is not exposed to the caller
// Shared between all hoverified code views
Expand Down Expand Up @@ -502,7 +508,7 @@ export function createHoverifier<C extends object, D, A>({
if (isPosition(position)) {
cell = dom.getCodeElementFromLineNumber(codeView, position.line, position.part)
if (cell) {
target = findElementWithOffset(cell, position.character)
target = findElementWithOffset(cell, position.character, tokenize)
if (target) {
part = dom.getDiffCodePart && dom.getDiffCodePart(target)
} else {
Expand Down Expand Up @@ -573,7 +579,7 @@ export function createHoverifier<C extends object, D, A>({
// placed in the middle of a token.
target:
position && isPosition(position)
? getTokenAtPosition(codeView, position, dom, position.part)
? getTokenAtPosition(codeView, position, dom, position.part, tokenize)
: target,
...rest,
})),
Expand Down Expand Up @@ -734,7 +740,7 @@ export function createHoverifier<C extends object, D, A>({
container.update({ hoveredTokenElement: undefined })
return
}
const token = getTokenAtPosition(codeView, highlightedRange.start, dom, part)
const token = getTokenAtPosition(codeView, highlightedRange.start, dom, part, tokenize)
container.update({ hoveredTokenElement: token })
if (!token) {
return
Expand Down Expand Up @@ -838,8 +844,10 @@ export function createHoverifier<C extends object, D, A>({
selectedPosition: position,
})
const codeElements = getCodeElementsInRange({ codeView, position, getCodeElementFromLineNumber })
for (const { element } of codeElements) {
convertNode(element)
if (tokenize) {
for (const { element } of codeElements) {
convertNode(element)
}
}
// Scroll into view
if (codeElements.length > 0) {
Expand Down
36 changes: 35 additions & 1 deletion src/token_position.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ describe('token_positions', () => {
})

describe('findElementWithOffset()', () => {
it('finds the correct token', () => {
it('finds the correct token (with tokenization)', () => {
const content = `${tabChar}if rv := contextGet(r, routeKey); rv != nil {`

const elems = [
Expand Down Expand Up @@ -122,6 +122,40 @@ describe('token_positions', () => {
}
})

it('finds the correct token (without tokenization)', () => {
const content = `<span role="presentation" style="padding-right: 0.1px;"><span class="cm-tab" role="presentation" cm-text=" "> </span><span class="cm-keyword">if</span> <span class="cm-variable">rv</span> :<span class="cm-operator">=</span> <span class="cm-variable">contextGet</span>(<span class="cm-variable">r</span>, <span class="cm-variable">varsKey</span>); <span class="cm-variable">rv</span> <span class="cm-operator">!=</span> <span class="cm-atom">nil</span> {</span>`

// Each offset is 3 more than the corresponding offset in the
// tokenized test above because this test case comes from Bitbucket
// where tabs are converted to spaces.
//
// The '(' and ' ' tokens are absent from this test because, on
// Bitbucket, punctuation characters are not wrapped in tags and the
// current offset-finding logic can't determine the offset for such
// tokens. One way to fix that is to use the CodeMirror API
// directly.
Copy link
Contributor

Choose a reason for hiding this comment

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

So we do need tokenization after all? I thought CodeMirror wrapped all tokens?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As far as I can tell, only punctuation characters are not wrapped. Normal tokens are wrapped. Since that covers 99% of use cases, I think it's ok. WDYT?

Copy link
Contributor

Choose a reason for hiding this comment

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

👍

const elems = [
{
offset: 14,
token: 'contextGet',
},
{
offset: 5,
token: 'if',
},
]

const elem = dom.createElementFromString(content)

for (const { offset, token } of elems) {
const tokenElem = findElementWithOffset(elem, offset, false)

expect(tokenElem).to.not.equal(undefined)

expect(tokenElem!.textContent).to.equal(token)
}
})

it('returns undefined for invalid offsets', () => {
const content = 'Hello, World!'

Expand Down
17 changes: 12 additions & 5 deletions src/token_position.ts
Original file line number Diff line number Diff line change
Expand Up @@ -252,9 +252,15 @@ export const getTextNodes = (node: Node): Node[] => {
* @param codeElement the element containing syntax highlighted code
* @param offset character offset (1-indexed)
*/
export function findElementWithOffset(codeElement: HTMLElement, offset: number): HTMLElement | undefined {
// Without being converted first, finding the position is inaccurate
convertCodeElementIdempotent(codeElement)
export function findElementWithOffset(
codeElement: HTMLElement,
offset: number,
tokenize = true
): HTMLElement | undefined {
if (tokenize) {
// Without being converted first, finding the position is inaccurate
convertCodeElementIdempotent(codeElement)
}

const textNodes = getTextNodes(codeElement)

Expand Down Expand Up @@ -475,7 +481,8 @@ export const getTokenAtPosition = (
getCodeElementFromLineNumber,
isFirstCharacterDiffIndicator,
}: Pick<DOMFunctions, 'getCodeElementFromLineNumber' | 'isFirstCharacterDiffIndicator'>,
part?: DiffPart
part?: DiffPart,
tokenize = true,
): HTMLElement | undefined => {
const codeElement = getCodeElementFromLineNumber(codeView, line, part)
if (!codeElement) {
Expand All @@ -486,5 +493,5 @@ export const getTokenAtPosition = (
character++
}

return findElementWithOffset(codeElement, character)
return findElementWithOffset(codeElement, character, tokenize)
}