-
Notifications
You must be signed in to change notification settings - Fork 99
Enables a hit count indicator to show number of times a line has been hit #491
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
base: master
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,6 +2,7 @@ import { Section } from "lcov-parse"; | |
| import { | ||
| Range, | ||
| TextEditor, | ||
| DecorationOptions | ||
| } from "vscode"; | ||
| import { Config } from "../extension/config"; | ||
| import { SectionFinder } from "./sectionfinder"; | ||
|
|
@@ -10,11 +11,13 @@ export interface ICoverageLines { | |
| full: Range[]; | ||
| partial: Range[]; | ||
| none: Range[]; | ||
| hitCounts: Map<number, number>; | ||
| } | ||
|
|
||
| export class Renderer { | ||
| private configStore: Config; | ||
| private sectionFinder: SectionFinder; | ||
| private maxHitCount: number = 0; | ||
|
|
||
| constructor( | ||
| configStore: Config, | ||
|
|
@@ -37,6 +40,7 @@ export class Renderer { | |
| full: [], | ||
| none: [], | ||
| partial: [], | ||
| hitCounts: new Map(), | ||
| }; | ||
|
|
||
| textEditors.forEach((textEditor) => { | ||
|
|
@@ -49,6 +53,8 @@ export class Renderer { | |
| coverageLines.full = []; | ||
| coverageLines.none = []; | ||
| coverageLines.partial = []; | ||
| coverageLines.hitCounts = new Map(); | ||
| this.maxHitCount = 0; | ||
|
|
||
| // find the section(s) (or undefined) by looking relatively at each workspace | ||
| // users can also optional use absolute instead of relative for this | ||
|
|
@@ -73,6 +79,9 @@ export class Renderer { | |
| this.configStore.partialCoverageDecorationType, | ||
| [], | ||
| ); | ||
|
|
||
| // Clean up hit count decorations if they exist | ||
| editor.setDecorations(this.configStore.hitCountDecorationType, []); | ||
| } | ||
|
|
||
| public setDecorationsForEditor( | ||
|
|
@@ -92,6 +101,64 @@ export class Renderer { | |
| this.configStore.partialCoverageDecorationType, | ||
| coverage.partial, | ||
| ); | ||
|
|
||
| // Apply hit count decorations if enabled | ||
| if (this.configStore.showHitCounts) { | ||
| this.setHitCountDecorations(editor, coverage); | ||
| } | ||
| } | ||
|
|
||
| private setHitCountDecorations( | ||
| editor: TextEditor, | ||
| coverage: ICoverageLines, | ||
| ) { | ||
| const hitCountDecorations: DecorationOptions[] = []; | ||
|
|
||
| const paddingWidth = this.maxHitCount.toString().length; | ||
|
|
||
| const addEmptyPadding = (startLine: number, endLine: number) => { | ||
| for (let line = startLine; line <= endLine; line++) { | ||
| hitCountDecorations.push({ | ||
| range: new Range(line, 0, line, 0), | ||
| renderOptions: { | ||
| before: { | ||
| // We use this special invisible character for the margin since spaces are trimmed | ||
| contentText: '\u00A0'.repeat(paddingWidth) + '\u00A0', | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ahh this is crafty, nice find / work around! |
||
| } | ||
| } | ||
| }); | ||
| } | ||
| }; | ||
|
|
||
| let lastLineNumber = -1; | ||
|
|
||
| // Create decorations for all lines with coverage data | ||
| coverage.hitCounts.forEach((hitCount, lineNumber) => { | ||
| if (lineNumber > lastLineNumber + 1) { | ||
| addEmptyPadding(lastLineNumber + 1, lineNumber - 1); | ||
| } | ||
|
|
||
| const range = new Range(lineNumber, 0, lineNumber, 0); | ||
| const paddedHitCount = hitCount.toString().padStart(paddingWidth, '\u00A0'); | ||
|
||
|
|
||
| hitCountDecorations.push({ | ||
| range, | ||
| renderOptions: { | ||
| before: { | ||
| contentText: paddedHitCount + '\u00A0', | ||
| } | ||
| } | ||
| }); | ||
|
|
||
| lastLineNumber = lineNumber; | ||
| }); | ||
|
|
||
| // Fill gap at end of file if needed | ||
| if (lastLineNumber < editor.document.lineCount - 1) { | ||
| addEmptyPadding(lastLineNumber + 1, editor.document.lineCount - 1); | ||
| } | ||
|
|
||
| editor.setDecorations(this.configStore.hitCountDecorationType, hitCountDecorations); | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -120,6 +187,11 @@ export class Renderer { | |
| .filter((detail) => detail.line > 0) | ||
| .forEach((detail) => { | ||
| const lineRange = new Range(detail.line - 1, 0, detail.line - 1, 0); | ||
|
|
||
| // Store hit count for this line | ||
| coverageLines.hitCounts.set(detail.line - 1, detail.hit); | ||
| this.maxHitCount = Math.max(this.maxHitCount, detail.hit); | ||
ryanluker marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| if (detail.hit > 0) { | ||
| // Evaluates to true if at least one element in range is equal to LineRange | ||
| if (coverageLines.none.some((range) => range.isEqual(lineRange))) { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,11 +16,13 @@ export class Config { | |
| public fullCoverageDecorationType!: TextEditorDecorationType; | ||
| public partialCoverageDecorationType!: TextEditorDecorationType; | ||
| public noCoverageDecorationType!: TextEditorDecorationType; | ||
| public hitCountDecorationType!: TextEditorDecorationType; | ||
| public showStatusBarToggler!: boolean; | ||
| public ignoredPathGlobs!: string; | ||
| public remotePathResolve!: string[]; | ||
| public manualCoverageFilePaths!: string[]; | ||
| public watchOnActivate!: boolean; | ||
| public showHitCounts!: boolean; | ||
|
|
||
| private context: ExtensionContext; | ||
|
|
||
|
|
@@ -71,6 +73,8 @@ export class Config { | |
| const showGutterCoverage = rootConfig.get("showGutterCoverage") as string; | ||
| const showLineCoverage = rootConfig.get("showLineCoverage") as string; | ||
| const showRulerCoverage = rootConfig.get("showRulerCoverage") as string; | ||
| this.showHitCounts = rootConfig.get("showHitCounts") as boolean; | ||
| const hitCountColor = rootConfig.get("hitCountColor") as string; | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should the colour be coupled to the gutter / line colour? Or is there a good reason that the hit count could be colourized 🤔.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not really, it was mostly just easier and more flexible that way. But coupling it to the gutter or line colour also makes sense So just to confirm, are you suggesting that the
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can leave it as is and let people choose, but we will need a default and that could be white for dark themes and black for light themes. (if we don't want dynamic default based on the currently active colour theme, I guess we can figure out something that works for both light / dark themes?) |
||
|
|
||
| const makeIcon = (colour: string): string | Uri => { | ||
| colour = colour | ||
|
|
@@ -138,12 +142,19 @@ export class Config { | |
| overviewRulerLane: OverviewRulerLane.Full, | ||
| }; | ||
|
|
||
| const hitCountDecoration: DecorationRenderOptions = { | ||
| before: { | ||
| color: hitCountColor, | ||
| } | ||
| }; | ||
|
|
||
| this.cleanupEmptyGutterIcons(fullDecoration, partialDecoration, noDecoration); | ||
|
|
||
| // Generate decorations | ||
| this.noCoverageDecorationType = window.createTextEditorDecorationType(noDecoration); | ||
| this.partialCoverageDecorationType = window.createTextEditorDecorationType(partialDecoration); | ||
| this.fullCoverageDecorationType = window.createTextEditorDecorationType(fullDecoration); | ||
| this.hitCountDecorationType = window.createTextEditorDecorationType(hitCountDecoration); | ||
|
|
||
| // Assign the key and resolved fragment | ||
| this.remotePathResolve = rootConfig.get("remotePathResolve") as string[]; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This feature is pretty sweet (imo), it would be nice to have on my default, but I am not sure if developers would immediately "get" what it was for 🤔.
@mattseddon and I chatted about small tutorials or toasts that prompt these kinda features for users to enable but we never got around to building any of that out...
#333
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! Yes, I totally agree that it might be confusing for newcomers. Having some sort of intro is a good idea imo. before enabling this feature by default
On the other hand, I have nothing against enabling it by default now
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's leave it off for the first release, and then we can turn it on for all in the one after. We could also consider maybe a tutorial / mention in the main
readme.mdshowing off the feature (currently the gif / image is pretty out of date and could use a new short highlight walkthrough).