Render machine annotations for multiple lines or columns

app/assets/javascripts/components/annotations/code_listing_row.ts

@@ -4,11 +4,15 @@ import { html, TemplateResult } from "lit";
 import { unsafeHTML } from "lit/directives/unsafe-html.js";
 import "components/annotations/hidden_annotations_dot";
 import "components/annotations/annotations_cell";
+import "components/annotations/machine_annotation_marker";
 import { i18nMixin } from "components/meta/i18n_mixin";
 import { initTooltips } from "util.js";
 import { PropertyValues } from "@lit/reactive-element";
 import { userState } from "state/Users";
 import { annotationState } from "state/Annotations";
+import { MachineAnnotationData, machineAnnotationState } from "state/MachineAnnotations";
+import { MachineAnnotationMarker } from "components/annotations/machine_annotation_marker";
+import { wrapRangesInHtml, range } from "mark";
 
 /**
  * This component represents a row in the code listing.
@@ -30,6 +34,45 @@ export class CodeListingRow extends i18nMixin(ShadowlessLitElement) {
     @property({ state: true })
     showForm: boolean;
 
+    /**
+     * Calculates the range of the code that is covered by the given annotation.
+     * If the annotation spans multiple lines, the range will be the whole line unless this is the first or last line.
+     * In that case, the range will be the part of the line that is covered by the annotation.
+     * @param annotation The annotation to calculate the range for.
+     */
+    getRangeFromAnnotation(annotation: MachineAnnotationData): range {
+        const rowsLength = annotation.rows ?? 1;
+        const lastRow = annotation.row + rowsLength ?? 0;
+        const firstRow = annotation.row + 1 ?? 0;
+
+        let start = 0;
+        if (this.row === firstRow) {
+            start = annotation.column || 0;
+        }
+
+        let length = Infinity;
+        if (this.row === lastRow) {
+            if (annotation.column !== undefined && annotation.column !== null) {
+                length = annotation.columns || 0;
+            }
+        }
+
+        return { start: start, length: length, data: annotation };
+    }
+
+    get wrappedCode(): string {
+        return wrapRangesInHtml(
+            this.renderedCode,
+            this.machineAnnotationToMark.map(a => this.getRangeFromAnnotation(a)),
+            "d-machine-annotation-marker",
+            (node: MachineAnnotationMarker, range) => {
+                // these nodes will be recompiled to html, so we need to store the data in a json string
+                const annotations = JSON.parse(node.getAttribute("annotations")) || [];
+                annotations.push(range.data);
+                node.setAttribute("annotations", JSON.stringify(annotations));
+            });
+    }
+
     firstUpdated(_changedProperties: PropertyValues): void {
         super.firstUpdated(_changedProperties);
         initTooltips(this);
@@ -43,6 +86,10 @@ export class CodeListingRow extends i18nMixin(ShadowlessLitElement) {
         return annotationState.isQuestionMode ? I18n.t("js.annotations.options.add_question") : I18n.t("js.annotations.options.add_annotation");
     }
 
+    get machineAnnotationToMark(): MachineAnnotationData[] {
+        return machineAnnotationState.byMarkedLine.get(this.row) || [];
+    }
+
     render(): TemplateResult {
         return html`
                 <td class="rouge-gutter gl">
@@ -60,7 +107,7 @@ export class CodeListingRow extends i18nMixin(ShadowlessLitElement) {
                     <pre>${this.row}</pre>
                 </td>
                 <td class="rouge-code">
-                    <pre>${unsafeHTML(this.renderedCode)}</pre>
+                    <pre style="overflow: visible; display: inline-block;" >${unsafeHTML(this.wrappedCode)}</pre>
                     <d-annotations-cell .row=${this.row}
                                         .showForm="${this.showForm}"
                                         @close-form=${() => this.showForm = false}

app/assets/javascripts/components/annotations/machine_annotation_marker.ts

@@ -0,0 +1,68 @@
+import { customElement, property } from "lit/decorators.js";
+import { render, html, LitElement, TemplateResult, PropertyValues } from "lit";
+import { MachineAnnotationData } from "state/MachineAnnotations";
+import tippy, { Instance as Tippy, createSingleton } from "tippy.js";
+
+/**
+ * A marker that shows a tooltip with machine annotations.
+ *
+ * @prop {MachineAnnotationData[]} annotations The annotations to show in the tooltip.
+ *
+ * @element d-machine-annotation-marker
+ */
+@customElement("d-machine-annotation-marker")
+export class MachineAnnotationMarker extends LitElement {
+    @property({ type: Array })
+    annotations: MachineAnnotationData[];
+
+    static tippyInstances: Tippy[] = [];
+    static tippySingleton = createSingleton([], {
+        placement: "bottom-start",
+        interactive: true,
+        interactiveDebounce: 25,
+        delay: [500, 25],
+        offset: [-10, 2],
+        moveTransition: "transform 0.001s ease-out",
+        appendTo: () => document.querySelector(".code-table"),
+    });
+    static registerTippyInstance(instance: Tippy): void {
+        this.tippyInstances.push(instance);
+        this.tippySingleton.setInstances(this.tippyInstances);
+    }
+
+    protected firstUpdated(_changedProperties: PropertyValues): void {
+        super.firstUpdated(_changedProperties);
+        const tooltip = document.createElement("div");
+        tooltip.classList.add("marker-tooltip");
+        render(this.annotations.map(a => html`<d-machine-annotation .data=${a}></d-machine-annotation>`), tooltip);
+
+        const t = tippy(this, {
+            content: tooltip,
+        });
+        MachineAnnotationMarker.registerTippyInstance(t);
+    }
+
+    get markColor(): string {
+        const types = new Set(this.annotations.map(a => a.type));
+        if (types.has("error")) {
+            return "var(--error-color, red)";
+        } else if (types.has("warning")) {
+            return "var(--warning-color, yellow)";
+        } else if (types.has("info")) {
+            return "var(--info-color, blue)";
+        } else {
+            return undefined;
+        }
+    }
+
+    render(): TemplateResult {
+        return html`<style>
+            :host {
+                position: relative;
+                text-decoration: wavy underline ${this.markColor};
+            }
+        </style><slot><svg style="position: absolute; top: 9px; left: -7px" width="14" height="14" viewBox="0 0 24 24">
+            <path fill="${this.markColor}" d="M7.41 15.41L12 10.83l4.59 4.58L18 14l-6-6l-6 6l1.41 1.41Z"/>
+        </svg></slot>`;
+    }
+}

app/assets/javascripts/components/saved_annotations/saved_annotations_sidecard.ts

@@ -31,7 +31,6 @@ export class SavedAnnotationList extends ShadowlessLitElement {
     }
 
     render(): TemplateResult {
-        console.log("rendering saved annotations sidecard");
         return this.potentialSavedAnnotationsExist ? html`
             <div class="card">
                 <div class="card-supporting-text">

app/assets/javascripts/mark.ts

@@ -0,0 +1,133 @@
+export type range = { start: number, length: number, data?: unknown }; // data is optional
+type callback = (node: Node, range: range) => void;
+
+/**
+ * Returns an array of text nodes or empty wrapper nodes and their start and end indices in the root node.
+ * @param root The root node to search for text nodes.
+ * @param wrapper The type of wrapper to search for.
+ */
+function getTextNodes(root: Node, wrapper: string): { start: number, end: number, node: Text; }[] {
+    let val = "";
+    const nodes = [];
+    const iterator = document.createNodeIterator(root, NodeFilter.SHOW_ALL, node => {
+        if (node.nodeType === Node.TEXT_NODE) {
+            return NodeFilter.FILTER_ACCEPT;
+        } else if (node.nodeName.toUpperCase() === wrapper.toUpperCase() && node.childNodes.length === 0) {
+            // Accept empty wrapper nodes
+            return NodeFilter.FILTER_ACCEPT;
+        } else {
+            return NodeFilter.FILTER_REJECT;
+        }
+    });
+
+    let node;
+    while (node = iterator.nextNode()) {
+        nodes.push({
+            start: val.length,
+            end: (val += node.textContent).length,
+            node
+        });
+    }
+    return nodes;
+}
+
+/**
+ * Returns the closest wrapper node of the given type that is an ancestor of the given node.
+ * @param node The node to search for a wrapper.
+ * @param wrapper The type of wrapper to search for.
+ */
+function closestWrapper(node: Node, wrapper: string): Node | null {
+    let parent = node;
+    while (parent !== null) {
+        if (parent.nodeName.toUpperCase() === wrapper.toUpperCase()) {
+            return parent;
+        }
+        parent = parent.parentNode;
+    }
+    return null;
+}
+
+/**
+ * Wraps all elements in the given range of the root node in the given wrapper node.
+ * For each part of the range that is already wrapped in the given wrapper node, the callback is called with the existing wrapper node.
+ * For each part of the range that is not wrapped in the given wrapper node, the callback is called for the newly created wrapper node.
+ *
+ * If no text nodes are found in the root node, an empty wrapper node is created and the callback is called for it.
+ *
+ * @param root The root node to search for text nodes.
+ * @param range The range to wrap.
+ * @param wrapper The type of wrapper to create.
+ * @param callback The callback to call for each wrapper node.
+ */
+function wrapRange(root: Node, range: range, wrapper: string, callback: callback): void {
+    const start = range.start;
+    const end = start + range.length;
+    const nodes = getTextNodes(root, wrapper);
+
+    let wrappedLength = 0;
+    for (const node of nodes) {
+        if (node.end > start && node.start <= end && node.node.textContent !== "\n" || (range.length === 0 && node.end === start)) {
+            const closest = closestWrapper(node.node, wrapper);
+            if ( closest === null ) {
+                const splitStart = Math.max(0, start - node.start);
+                let nodeToWrap = node.node;
+                if (start > node.start) {
+                    nodeToWrap = node.node.splitText(splitStart);
+                }
+
+                if (node.end > end) {
+                    nodeToWrap.splitText(end - node.start - splitStart);
+                }
+                const wrapperNode = document.createElement(wrapper);
+                wrapperNode.textContent = nodeToWrap.textContent;
+                nodeToWrap.parentNode.replaceChild(wrapperNode, nodeToWrap);
+                callback(wrapperNode, range);
+
+                // Avoid needless wrapping of empty text nodes
+                wrappedLength += wrapperNode.textContent.length;
+                if (wrappedLength >= range.length) {
+                    return;
+                }
+            } else {
+                callback(closest, range);
+                // Avoid needless wrapping of empty text nodes
+                wrappedLength += closest.textContent.length;
+                if (wrappedLength >= range.length) {
+                    return;
+                }
+            }
+        }
+    }
+
+    if (nodes.length === 0) {
+        const wrapperNode = document.createElement(wrapper);
+        wrapperNode.textContent = root.textContent;
+        root.appendChild(wrapperNode);
+        callback(wrapperNode, range);
+    }
+}
+
+/**
+ * Wraps all elements in the given ranges of the root node in the given wrapper node.
+ * @param root The root node to search for text nodes.
+ * @param ranges The ranges to wrap.
+ * @param wrapper The type of wrapper to create.
+ * @param callback The callback to call for each wrapper node.
+ */
+function wrapRanges(root: Node, ranges: range[], wrapper: string, callback: callback): void {
+    ranges.forEach(range => wrapRange(root, range, wrapper, callback));
+}
+
+/**
+ * Wraps all elements in the given ranges of the given target string in the given wrapper node.
+ * @param target a html string whose text nodes should be wrapped
+ * @param ranges the ranges of the textcontent to wrap
+ * @param wrapper the type of wrapper to create
+ * @param callback the callback to call for each wrapper node
+ */
+export function wrapRangesInHtml(target: string, ranges: range[], wrapper: string, callback: callback): string {
+    const root = document.createElement("div");
+    root.innerHTML = target;
+    wrapRanges(root, ranges, wrapper, callback);
+    return root.innerHTML;
+}

app/assets/javascripts/state/Annotations.ts

@@ -6,7 +6,7 @@ import { stateProperty } from "state/state_system/StateProperty";
 export type AnnotationVisibilityOptions = "all" | "important" | "none";
 
 class AnnotationState extends State {
-    @stateProperty visibility: AnnotationVisibilityOptions = "all";
+    @stateProperty visibility: AnnotationVisibilityOptions = "important";
     @stateProperty isQuestionMode = false;
 
     isVisible(annotation: MachineAnnotationData | UserAnnotationData): boolean {

app/assets/javascripts/state/MachineAnnotations.ts

@@ -8,22 +8,36 @@ export interface MachineAnnotationData {
     text: string;
     row: number;
     externalUrl?: string | null;
+    rows?: number;
+    column?: number;
+    columns?: number;
 }
 
 class MachineAnnotationState extends State {
     @stateProperty public byLine = new StateMap<number, MachineAnnotationData[]>();
+    @stateProperty public byMarkedLine = new StateMap<number, MachineAnnotationData[]>();
     @stateProperty public count = 0;
 
     public setMachineAnnotations(annotations: MachineAnnotationData[]): void {
         this.count = annotations.length;
         this.byLine.clear();
+        this.byMarkedLine.clear();
         for (const annotation of annotations) {
-            const line = annotation.row + 1 ?? 0;
+            const markedLength = annotation.rows ?? 1;
+            const line = annotation.row + markedLength ?? 0;
             if (this.byLine.has(line)) {
                 this.byLine.get(line)?.push(annotation);
             } else {
                 this.byLine.set(line, [annotation]);
             }
+            for (let i = 1; i <= markedLength; i++) {
+                const markedLine = annotation.row + i;
+                if (this.byMarkedLine.has(markedLine)) {
+                    this.byMarkedLine.get(markedLine)?.push(annotation);
+                } else {
+                    this.byMarkedLine.set(markedLine, [annotation]);
+                }
+            }
         }
     }
 }

app/assets/stylesheets/components/code_listing.css.scss

@@ -373,3 +373,28 @@ $annotation-info: $info;
     }
   }
 }
+
+d-machine-annotation-marker {
+  --info-color: #{$annotation-info};
+  --warning-color: #{$annotation-warning};
+  --error-color: #{$annotation-error};
+}
+
+.marker-tooltip {
+  width: max-content;
+  white-space: initial;
+  color: $on-surface;
+  font-style: initial;
+  padding-left: 5px;
+  border-top-right-radius: 5px;
+  border-bottom-right-radius: 5px;
+  border: 1px solid $divider;
+  background-color: $content-bg;
+
+  .annotation {
+    margin: 4px 0;
+    border-right: none;
+    border-top: none;
+    border-bottom: none;
+  }
+}

app/runners/result_constructor.rb

@@ -121,8 +121,8 @@ def annotate_code(values)
       type: values[:type] || 'info',
       row: values[:row] || 0,
       rows: values[:rows] || 1,
-      column: values[:column] || 0,
-      columns: values[:columns] || 1,
+      column: values[:column] || nil,
+      columns: values[:columns] || nil,
       externalUrl: values[:externalUrl] || nil
     }
   end

package.json

@@ -39,6 +39,7 @@
     "lit": "2.7.2",
     "node-polyglot": "^2.5.0",
     "sass": "^1.62.0",
+    "tippy.js": "^6.3.7",
     "typescript": "^4.9.5",
     "webpack": "^5.79.0",
     "webpack-cli": "^4.9.2"
@@ -54,8 +55,8 @@
     "@types/jest": "^27.5.0",
     "@types/serviceworker": "^0.0.67",
     "@typescript-eslint/eslint-plugin": "^5.58.0",
-    "babel-plugin-istanbul": "^6.1.1",
     "@typescript-eslint/parser": "^5.58.0",
+    "babel-plugin-istanbul": "^6.1.1",
     "eslint": "^8.38.0",
     "eslint-config-google": "^0.14.0",
     "eslint-plugin-jest": "^27.2.1",