Skip to content
Open
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
1 change: 1 addition & 0 deletions example/example.code-workspace
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
],
"settings": {
"coverage-gutters.showLineCoverage": true,
"coverage-gutters.showHitCounts": true,
"coverage-gutters.coverageReportFileName": "index.html",
"coverage-gutters.remotePathResolve": ["/var/www/", "./"]
}
Expand Down
12 changes: 12 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,12 @@
"default": "rgba(163, 0, 0, 0.4)",
"description": "dark theme partial highlight for code coverage"
},
"coverage-gutters.hitCountColor": {
"type": "string",
"scope": "resource",
"default": "rgba(255, 255, 255, 0.8)",
"description": "color for hit count numbers in the gutter"
},
"coverage-gutters.showLineCoverage": {
"type": "boolean",
"default": false,
Expand All @@ -89,6 +95,12 @@
"default": true,
"description": "show or hide the gutter coverage"
},
"coverage-gutters.showHitCounts": {
"type": "boolean",
"scope": "resource",
"default": false,
Copy link
Owner

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

Copy link
Contributor Author

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

Copy link
Owner

Choose a reason for hiding this comment

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

On the other hand, I have nothing against enabling it by default now

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.md showing off the feature (currently the gif / image is pretty out of date and could use a new short highlight walkthrough).

"description": "show hit counts as numbers in the gutter"
},
"coverage-gutters.ignoredPathGlobs": {
"type": "string",
"scope": "resource",
Expand Down
73 changes: 73 additions & 0 deletions src/coverage-system/renderer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { Section } from "lcov-parse";
import {
Range,
TextEditor,
DecorationOptions
} from "vscode";
import { Config } from "../extension/config";
import { SectionFinder } from "./sectionfinder";
Expand All @@ -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,
Expand All @@ -37,6 +40,7 @@ export class Renderer {
full: [],
none: [],
partial: [],
hitCounts: new Map(),
};

textEditors.forEach((textEditor) => {
Expand All @@ -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
Expand All @@ -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(
Expand All @@ -92,6 +101,65 @@ 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',
Copy link
Owner

Choose a reason for hiding this comment

The 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 displayValue = hitCount > 1000 ? '>1000' : hitCount.toString();
const paddedHitCount = displayValue.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);
}

/**
Expand Down Expand Up @@ -120,6 +188,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);

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))) {
Expand Down
11 changes: 11 additions & 0 deletions src/extension/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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;
Copy link
Owner

Choose a reason for hiding this comment

The 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 🤔.

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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 hitCountColor should match the line colour (maybe a bit darker)? Or would you prefer the approach where we hardcode it, white for dark themes and black for light themes? Which one do you think works better? (or leave it as is and people can choose)

Copy link
Owner

Choose a reason for hiding this comment

The 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
Expand Down Expand Up @@ -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[];
Expand Down
Loading