Add Monaco diff-view, the saga ...
I set out to use Monaco to support the diff view. diff2html is lovely,
but there were a ton of usability improvements I wanted to make (line
numbers not making things double spaced, choosing which diff, editing
the right-hand side), and it seemed a dead end. Furthermore, Phabricator
and Gerrit's experience is that diffs should be shown file by file,
because you'll inevitably see a diff with a file that's too large, and
the GitHub PR view often breaks on big changes... so I wanted to show
files diff-by-diff, with "infinite" context when unchanged sections are
expanded. So...
Ultimately, all of this was sketch-coded over maybe 30 Sketch sessions.
I threw away a lot of branches. My git reflog is a superfund site.
Prompting whole-hog didn't work. Or, rather, it made significant
progress, but something very serious wouldn't work, and I couldn't
figure out what, and nor could Sketch.
Instead, I started by adding a new webcomponent that was just a
placeholder. Then, using https://rodydavis.com/posts/lit-monaco-editor,
I nudged Sketch into adding Monaco to it. Sketch pulled out:
You're right, I should properly read the blog post before implementing the
solution. Let me check the referenced blog post.
I worked heavily in the demo environment at first, but here I ran into
the issue that we have two different esbuild systems: one is vite and
one is esbuild.go, and they're configured differently enough.
Monaco is unusable and confusingly so when its CSS isn't loaded. The right
way to load it, I've found, is via
@import url('./static/monaco/min/vs/editor/editor.main.css');
I spent more time than I care to admit noticing that originally
this wasn't relative, and when we use a skaband setting, the
paths need to be relative-aware.
The paths to the various workers need to be similarly correctly placed.
Getting Sketch to build demo data but not put testing code into production
code was tricky. (I threw away a lot of efforts and factories and singletons...)
When I set out to do the git commit selection, I wanted to do a bunch of
backend /git/* handlers. These were easy enough to code in sketch. I had
to convince Sketch to put them in git_tools.go and not in the agent.
It doesn't really matter: these functions to parse git are pretty stateless,
but it's less work to have them separate. Sketch was mediocre at writing
tests for them. Did you know that our container has an older version
of git that doesn't have the same options to decorate ref names? Yeah, nor did
I.
Handling unstaged changes was fun. git diff --raw shows unstaged files
as having identity 0000. Ideally we'd be using jj and there'd be
a synthetic commit, but instead uncommitted-possible files are read
by content.
A real big challenge was getting the Monaco view to use the right vertical and
horizontal space. I did this many, many times. I don't claim to understand flex
and the virtual dom, and :host, and all the interactions. It would fix one
thing and break another. The chat window would shrink. The terminal would
shrink.
Screenshot support was excellent. I eventually added paste support just so
that I could expedite my workflow, and Sketch coded that easily on the first
pass with minor feedback.
I learned the hard way that Safari's support for WebComponents/shadow
dom in its web inspector is rough. See https://fediverse.zachleat.com/@zachleat/114518629612122858
I also learned the hard way that Chrome doesn't use fonts loaded in CSS
in a shadow dom. That's why the codicon font had to be in the global
style sheet.
Kudos to John Reese who kindly allowed me, a long time ago, to adapt a
shell script he had at work to look over diffs into https://github.com/philz/git-vimdiff.
That's the inspiration for having the "new code" be editable when you're
reviewing it; why shouldn't it be!?!
There are a handful of follow up tasks:
* We lose state when we switch to the Chat view and back.
* Need URL-based support for where we are.
* Maybe need shortcut keys to move between diffs and changes.
* Maybe need caching or look-ahead for downloading the next or previous
file.
* We spend too much vertical real estate on all the diff selections;
could we scroll it out of the way, collapse it, tighten it, etc.
* The workers sometimes throw errors into the console. I think they're
harmless and merely need to be caught and suppressed.
* Needing to commit changes when things are saved is weird. Should we
commit automatically? Amend the previous commit? Have a button for
that? Show the git dirty state?
* Our JS bundle is big. We could maybe delay loading the monaco bundle
to help.
Thanks for coming to my TED talk.
diff --git a/webui/src/web-components/sketch-diff2-view.ts b/webui/src/web-components/sketch-diff2-view.ts
new file mode 100644
index 0000000..5ac7459
--- /dev/null
+++ b/webui/src/web-components/sketch-diff2-view.ts
@@ -0,0 +1,536 @@
+import { css, html, LitElement } from "lit";
+import { customElement, property, state } from "lit/decorators.js";
+import "./sketch-monaco-view";
+import "./sketch-diff-range-picker";
+import "./sketch-diff-file-picker";
+import "./sketch-diff-empty-view";
+import { GitDiffFile, GitDataService, DefaultGitDataService } from "./git-data-service";
+import { DiffRange } from "./sketch-diff-range-picker";
+
+/**
+ * A component that displays diffs using Monaco editor with range and file pickers
+ */
+@customElement("sketch-diff2-view")
+export class SketchDiff2View extends LitElement {
+ /**
+ * Handles comment events from the Monaco editor and forwards them to the chat input
+ * using the same event format as the original diff view for consistency.
+ */
+ private handleMonacoComment(event: CustomEvent) {
+ try {
+ // Validate incoming data
+ if (!event.detail || !event.detail.formattedComment) {
+ console.error('Invalid comment data received');
+ return;
+ }
+
+ // Create and dispatch event using the standardized format
+ const commentEvent = new CustomEvent('diff-comment', {
+ detail: { comment: event.detail.formattedComment },
+ bubbles: true,
+ composed: true
+ });
+
+ this.dispatchEvent(commentEvent);
+ } catch (error) {
+ console.error('Error handling Monaco comment:', error);
+ }
+ }
+
+ /**
+ * Handle save events from the Monaco editor
+ */
+ private async handleMonacoSave(event: CustomEvent) {
+ try {
+ // Validate incoming data
+ if (!event.detail || !event.detail.path || event.detail.content === undefined) {
+ console.error('Invalid save data received');
+ return;
+ }
+
+ const { path, content } = event.detail;
+
+ // Get Monaco view component
+ const monacoView = this.shadowRoot?.querySelector('sketch-monaco-view');
+ if (!monacoView) {
+ console.error('Monaco view not found');
+ return;
+ }
+
+ try {
+ await this.gitService?.saveFileContent(path, content);
+ console.log(`File saved: ${path}`);
+ (monacoView as any).notifySaveComplete(true);
+ } catch (error) {
+ console.error(`Error saving file: ${error instanceof Error ? error.message : String(error)}`);
+ (monacoView as any).notifySaveComplete(false);
+ }
+ } catch (error) {
+ console.error('Error handling save:', error);
+ }
+ }
+ @property({ type: String })
+ initialCommit: string = "";
+
+ // The commit to show - used when showing a specific commit from timeline
+ @property({ type: String })
+ commit: string = "";
+
+ @property({ type: String })
+ selectedFilePath: string = "";
+
+ @state()
+ private files: GitDiffFile[] = [];
+
+ @state()
+ private currentRange: DiffRange = { type: 'range', from: '', to: 'HEAD' };
+
+ @state()
+ private originalCode: string = "";
+
+ @state()
+ private modifiedCode: string = "";
+
+ @state()
+ private isRightEditable: boolean = false;
+
+ @state()
+ private loading: boolean = false;
+
+ @state()
+ private error: string | null = null;
+
+ static styles = css`
+ :host {
+ display: flex;
+ height: 100%;
+ flex: 1;
+ flex-direction: column;
+ min-height: 0; /* Critical for flex child behavior */
+ overflow: hidden;
+ position: relative; /* Establish positioning context */
+ }
+
+ .controls {
+ padding: 8px 16px;
+ border-bottom: 1px solid var(--border-color, #e0e0e0);
+ background-color: var(--background-light, #f8f8f8);
+ flex-shrink: 0; /* Prevent controls from shrinking */
+ }
+
+ .controls-container {
+ display: flex;
+ flex-direction: column;
+ gap: 12px;
+ }
+
+ .range-row {
+ width: 100%;
+ display: flex;
+ }
+
+ .file-row {
+ width: 100%;
+ display: flex;
+ justify-content: space-between;
+ align-items: center;
+ gap: 10px;
+ }
+
+ sketch-diff-range-picker {
+ width: 100%;
+ }
+
+ sketch-diff-file-picker {
+ flex: 1;
+ }
+
+ .view-toggle-button {
+ background-color: #f0f0f0;
+ border: 1px solid #ccc;
+ border-radius: 4px;
+ padding: 6px 12px;
+ font-size: 12px;
+ cursor: pointer;
+ white-space: nowrap;
+ transition: background-color 0.2s;
+ }
+
+ .view-toggle-button:hover {
+ background-color: #e0e0e0;
+ }
+
+ .diff-container {
+ flex: 1;
+ overflow: hidden;
+ display: flex;
+ flex-direction: column;
+ min-height: 0; /* Critical for flex child to respect parent height */
+ position: relative; /* Establish positioning context */
+ height: 100%; /* Take full height */
+ }
+
+ .diff-content {
+ flex: 1;
+ overflow: hidden;
+ min-height: 0; /* Required for proper flex behavior */
+ display: flex; /* Required for child to take full height */
+ position: relative; /* Establish positioning context */
+ height: 100%; /* Take full height */
+ }
+
+ .loading, .empty-diff {
+ display: flex;
+ align-items: center;
+ justify-content: center;
+ height: 100%;
+ font-family: var(--font-family, system-ui, sans-serif);
+ }
+
+ .empty-diff {
+ color: var(--text-secondary-color, #666);
+ font-size: 16px;
+ text-align: center;
+ }
+
+ .error {
+ color: var(--error-color, #dc3545);
+ padding: 16px;
+ font-family: var(--font-family, system-ui, sans-serif);
+ }
+
+ sketch-monaco-view {
+ --editor-width: 100%;
+ --editor-height: 100%;
+ flex: 1; /* Make Monaco view take full height */
+ display: flex; /* Required for child to take full height */
+ position: absolute; /* Absolute positioning to take full space */
+ top: 0;
+ left: 0;
+ right: 0;
+ bottom: 0;
+ height: 100%; /* Take full height */
+ width: 100%; /* Take full width */
+ }
+ `;
+
+ @property({ attribute: false, type: Object })
+ gitService!: GitDataService;
+
+ // The gitService must be passed from parent to ensure proper dependency injection
+
+ constructor() {
+ super();
+ console.log('SketchDiff2View initialized');
+
+ // Fix for monaco-aria-container positioning
+ // Add a global style to ensure proper positioning of aria containers
+ const styleElement = document.createElement('style');
+ styleElement.textContent = `
+ .monaco-aria-container {
+ position: absolute !important;
+ top: 0 !important;
+ left: 0 !important;
+ width: 1px !important;
+ height: 1px !important;
+ overflow: hidden !important;
+ clip: rect(1px, 1px, 1px, 1px) !important;
+ white-space: nowrap !important;
+ margin: 0 !important;
+ padding: 0 !important;
+ border: 0 !important;
+ z-index: -1 !important;
+ }
+ `;
+ document.head.appendChild(styleElement);
+ }
+
+ connectedCallback() {
+ super.connectedCallback();
+ // Initialize with default range and load data
+ // Get base commit if not set
+ if (this.currentRange.type === 'range' && !('from' in this.currentRange && this.currentRange.from)) {
+ this.gitService.getBaseCommitRef().then(baseRef => {
+ this.currentRange = { type: 'range', from: baseRef, to: 'HEAD' };
+ this.loadDiffData();
+ }).catch(error => {
+ console.error('Error getting base commit ref:', error);
+ // Use default range
+ this.loadDiffData();
+ });
+ } else {
+ this.loadDiffData();
+ }
+ }
+
+ // Toggle hideUnchangedRegions setting
+ @state()
+ private hideUnchangedRegionsEnabled: boolean = true;
+
+ // Toggle hideUnchangedRegions setting
+ private toggleHideUnchangedRegions() {
+ this.hideUnchangedRegionsEnabled = !this.hideUnchangedRegionsEnabled;
+
+ // Get the Monaco view component
+ const monacoView = this.shadowRoot?.querySelector('sketch-monaco-view');
+ if (monacoView) {
+ (monacoView as any).toggleHideUnchangedRegions(this.hideUnchangedRegionsEnabled);
+ }
+ }
+
+ render() {
+ return html`
+ <div class="controls">
+ <div class="controls-container">
+ <div class="range-row">
+ <sketch-diff-range-picker
+ .gitService="${this.gitService}"
+ @range-change="${this.handleRangeChange}"
+ ></sketch-diff-range-picker>
+ </div>
+
+ <div class="file-row">
+ <sketch-diff-file-picker
+ .files="${this.files}"
+ .selectedPath="${this.selectedFilePath}"
+ @file-selected="${this.handleFileSelected}"
+ ></sketch-diff-file-picker>
+
+ <div style="display: flex; gap: 8px;">
+ ${this.isRightEditable ? html`
+ <div class="editable-indicator" title="This file is editable">
+ <span style="padding: 6px 12px; background-color: #e9ecef; border-radius: 4px; font-size: 12px; color: #495057;">
+ Editable
+ </span>
+ </div>
+ ` : ''}
+ <button
+ class="view-toggle-button"
+ @click="${this.toggleHideUnchangedRegions}"
+ title="${this.hideUnchangedRegionsEnabled ? 'Expand All' : 'Hide Unchanged'}"
+ >
+ ${this.hideUnchangedRegionsEnabled ? 'Expand All' : 'Hide Unchanged'}
+ </button>
+ </div>
+ </div>
+ </div>
+ </div>
+
+ <div class="diff-container">
+ <div class="diff-content">
+ ${this.renderDiffContent()}
+ </div>
+ </div>
+ `;
+ }
+
+ renderDiffContent() {
+ if (this.loading) {
+ return html`<div class="loading">Loading diff...</div>`;
+ }
+
+ if (this.error) {
+ return html`<div class="error">${this.error}</div>`;
+ }
+
+ if (this.files.length === 0) {
+ return html`<sketch-diff-empty-view></sketch-diff-empty-view>`;
+ }
+
+ if (!this.selectedFilePath) {
+ return html`<div class="loading">Select a file to view diff</div>`;
+ }
+
+ return html`
+ <sketch-monaco-view
+ .originalCode="${this.originalCode}"
+ .modifiedCode="${this.modifiedCode}"
+ .originalFilename="${this.selectedFilePath}"
+ .modifiedFilename="${this.selectedFilePath}"
+ ?readOnly="${!this.isRightEditable}"
+ ?editable-right="${this.isRightEditable}"
+ @monaco-comment="${this.handleMonacoComment}"
+ @monaco-save="${this.handleMonacoSave}"
+ ></sketch-monaco-view>
+ `;
+ }
+
+ /**
+ * Load diff data for the current range
+ */
+ async loadDiffData() {
+ this.loading = true;
+ this.error = null;
+
+ try {
+ // Initialize files as empty array if undefined
+ if (!this.files) {
+ this.files = [];
+ }
+
+ // Load diff data based on the current range type
+ if (this.currentRange.type === 'single') {
+ this.files = await this.gitService.getCommitDiff(this.currentRange.commit);
+ } else {
+ this.files = await this.gitService.getDiff(this.currentRange.from, this.currentRange.to);
+ }
+
+ // Ensure files is always an array, even when API returns null
+ if (!this.files) {
+ this.files = [];
+ }
+
+ // If we have files, select the first one and load its content
+ if (this.files.length > 0) {
+ const firstFile = this.files[0];
+ this.selectedFilePath = firstFile.path;
+
+ // Directly load the file content, especially important when there's only one file
+ // as sometimes the file-selected event might not fire in that case
+ this.loadFileContent(firstFile);
+ } else {
+ // No files to display - reset the view to initial state
+ this.selectedFilePath = '';
+ this.originalCode = '';
+ this.modifiedCode = '';
+ }
+ } catch (error) {
+ console.error('Error loading diff data:', error);
+ this.error = `Error loading diff data: ${error.message}`;
+ // Ensure files is an empty array when an error occurs
+ this.files = [];
+ // Reset the view to initial state
+ this.selectedFilePath = '';
+ this.originalCode = '';
+ this.modifiedCode = '';
+ } finally {
+ this.loading = false;
+ }
+ }
+
+ /**
+ * Load the content of the selected file
+ */
+ async loadFileContent(file: GitDiffFile) {
+ this.loading = true;
+ this.error = null;
+
+ try {
+ let fromCommit: string;
+ let toCommit: string;
+ let isUnstagedChanges = false;
+
+ // Determine the commits to compare based on the current range
+ if (this.currentRange.type === 'single') {
+ fromCommit = `${this.currentRange.commit}^`;
+ toCommit = this.currentRange.commit;
+ } else {
+ fromCommit = this.currentRange.from;
+ toCommit = this.currentRange.to;
+ // Check if this is an unstaged changes view
+ isUnstagedChanges = toCommit === '';
+ }
+
+ // Set editability based on whether we're showing uncommitted changes
+ this.isRightEditable = isUnstagedChanges;
+
+ // Load the original code based on file status
+ if (file.status === 'A') {
+ // Added file: empty original
+ this.originalCode = '';
+ } else {
+ // For modified, renamed, or deleted files: load original content
+ this.originalCode = await this.gitService.getFileContent(file.old_hash || '');
+ }
+
+ // For modified code, always use working copy when editable
+ if (this.isRightEditable) {
+ try {
+ // Always use working copy when editable, regardless of diff status
+ // This ensures we have the latest content even if the diff hasn't been refreshed
+ this.modifiedCode = await this.gitService.getWorkingCopyContent(file.path);
+ } catch (error) {
+ if (file.status === 'D') {
+ // For deleted files, silently use empty content
+ console.warn(`Could not get working copy for deleted file ${file.path}, using empty content`);
+ this.modifiedCode = '';
+ } else {
+ // For any other file status, propagate the error
+ console.error(`Failed to get working copy for ${file.path}:`, error);
+ throw error; // Rethrow to be caught by the outer try/catch
+ }
+ }
+ } else {
+ // For non-editable view, use git content based on file status
+ if (file.status === 'D') {
+ // Deleted file: empty modified
+ this.modifiedCode = '';
+ } else {
+ // Added/modified/renamed: use the content from git
+ this.modifiedCode = await this.gitService.getFileContent(file.new_hash || '');
+ }
+ }
+
+ // Don't make deleted files editable
+ if (file.status === 'D') {
+ this.isRightEditable = false;
+ }
+ } catch (error) {
+ console.error('Error loading file content:', error);
+ this.error = `Error loading file content: ${error.message}`;
+ this.isRightEditable = false;
+ } finally {
+ this.loading = false;
+ }
+ }
+
+ /**
+ * Handle range change event from the range picker
+ */
+ handleRangeChange(event: CustomEvent) {
+ const { range } = event.detail;
+ console.log('Range changed:', range);
+ this.currentRange = range;
+
+ // Load diff data for the new range
+ this.loadDiffData();
+ }
+
+ /**
+ * Handle file selection event from the file picker
+ */
+ handleFileSelected(event: CustomEvent) {
+ const file = event.detail.file as GitDiffFile;
+ this.selectedFilePath = file.path;
+ this.loadFileContent(file);
+ }
+
+ /**
+ * Refresh the diff view by reloading commits and diff data
+ *
+ * This is called when the Monaco diff tab is activated to ensure:
+ * 1. Branch information from git/recentlog is current (branches can change frequently)
+ * 2. The diff content is synchronized with the latest repository state
+ * 3. Users always see up-to-date information without manual refresh
+ */
+ refreshDiffView() {
+ // First refresh the range picker to get updated branch information
+ const rangePicker = this.shadowRoot?.querySelector('sketch-diff-range-picker');
+ if (rangePicker) {
+ (rangePicker as any).loadCommits();
+ }
+
+ if (this.commit) {
+ this.currentRange = { type: 'single', commit: this.commit };
+ }
+
+ // Then reload diff data based on the current range
+ this.loadDiffData();
+ }
+}
+
+declare global {
+ interface HTMLElementTagNameMap {
+ "sketch-diff2-view": SketchDiff2View;
+ }
+}