Three-way diffs (#1147)

* Misc cleanups

* Three-way diffs

Fixes #93.

* Setting for three-way diffing mode

* fixes

* Error checking

* wording

* fix

* Help text

* Style the 2/3 button

* yarn lint

* make a smidge smaller

* make the 2/3 a tiny bit smaller

* Add -3/-b flag to editor settings

---------

Co-authored-by: Mark Street <streetster@gmail.com>
This commit is contained in:
Simon Lindholm
2024-03-26 07:57:16 +01:00
committed by GitHub
parent bb4a367de1
commit ae05e362f7
12 changed files with 344 additions and 44 deletions
+1 -1
View File
@@ -102,7 +102,7 @@ poetry run python manage.py makemigrations
poetry run python manage.py migrate
```
### Frontend styling
### Frontend styling
We use Tailwind CSS with Radix UI colors. Each color is on a scale from 1 to 12 (inclusive), each with [a well-defined meaning](https://www.radix-ui.com/docs/colors/palette-composition/understanding-the-scale).
-1
View File
@@ -1,6 +1,5 @@
/// <reference types="next" />
/// <reference types="next/image-types/global" />
/// <reference types="next/navigation-types/compat/navigation" />
// NOTE: This file should not be edited
// see https://nextjs.org/docs/basic-features/typescript for more information.
@@ -0,0 +1,54 @@
import { ReactNode, useId } from "react"
function RadioButton({ name, value, checked, onChange, option }: { name: string, value: string, checked: boolean, onChange: (value: string) => void, option: Option }) {
const id = useId()
return <div className="flex gap-2">
<div>
<input
id={id}
name={name}
value={value}
type="radio"
checked={checked}
onChange={evt => onChange(evt.target.value)}
/>
</div>
<div className="grow">
<label htmlFor={id} className="select-none font-semibold">{option.label}</label>
{option.description && <div className="text-sm text-gray-11">{option.description}</div>}
{option.children && <div className="pt-3">
{option.children}
</div>}
</div>
</div>
}
export type Option = {
label: ReactNode
description?: ReactNode
children?: ReactNode
}
export type Props = {
value: string
onChange: (value: string) => void
options: { [key: string]: Option }
}
export default function RadioList({ value, onChange, options }: Props) {
const name = useId()
return <div className="p-1">
{Object.keys(options).map(key =>
<RadioButton
name={name}
key={key}
value={key}
checked={key === value}
option={options[key]}
onChange={onChange}
/>
)}
</div>
}
@@ -2,18 +2,21 @@
import { useEffect, useRef, useState } from "react"
import LoadingSpinner from "@/components/loading.svg"
import * as settings from "@/lib/settings"
import { ThreeWayDiffBase, useAutoRecompileSetting, useAutoRecompileDelaySetting, useMatchProgressBarEnabled,
useLanguageServerEnabled, useVimModeEnabled, useThreeWayDiffBase } from "@/lib/settings"
import Checkbox from "../Checkbox"
import RadioList from "../RadioList"
import Section from "../Section"
import SliderField from "../SliderField"
export default function EditorSettings() {
const [autoRecompile, setAutoRecompile] = settings.useAutoRecompileSetting()
const [autoRecompileDelay, setAutoRecompileDelay] = settings.useAutoRecompileDelaySetting()
const [matchProgressBarEnabled, setMatchProgressBarEnabled] = settings.useMatchProgressBarEnabled()
const [languageServerEnabled, setLanguageServerEnabled] = settings.useLanguageServerEnabled()
const [vimModeEnabled, setVimModeEnabled] = settings.useVimModeEnabled()
const [autoRecompile, setAutoRecompile] = useAutoRecompileSetting()
const [autoRecompileDelay, setAutoRecompileDelay] = useAutoRecompileDelaySetting()
const [matchProgressBarEnabled, setMatchProgressBarEnabled] = useMatchProgressBarEnabled()
const [languageServerEnabled, setLanguageServerEnabled] = useLanguageServerEnabled()
const [vimModeEnabled, setVimModeEnabled] = useVimModeEnabled()
const [threeWayDiffBase, setThreeWayDiffBase] = useThreeWayDiffBase()
const [downloadingLanguageServer, setDownloadingLanguageServer] = useState(false)
@@ -39,6 +42,11 @@ export default function EditorSettings() {
}
}, [languageServerEnabled])
const threeWayDiffOptions = {
[ThreeWayDiffBase.SAVED]: { label: <div>Latest save ( <span className="font-mono text-gray-11">diff.py -b</span> )</div> },
[ThreeWayDiffBase.PREV]: { label: <div>Previous result ( <span className="font-mono text-gray-11">diff.py -3</span> )</div> },
}
return <>
<Section title="Automatic compilation">
<Checkbox
@@ -61,6 +69,18 @@ export default function EditorSettings() {
</div>
</Checkbox>
</Section>
<Section title="Three-way diffing target">
<div className="text-gray-11">
When enabling three-way diffing for a scratch, let the third column show a diff against:
</div>
<RadioList
value={threeWayDiffBase}
onChange={(value: string) => {
setThreeWayDiffBase(value as ThreeWayDiffBase)
}}
options={threeWayDiffOptions}
/>
</Section>
<Section title="Match progress bar">
<Checkbox
checked={matchProgressBarEnabled}
@@ -1,10 +1,12 @@
import { useEffect, useRef, useState } from "react"
import { useMemo, useRef, useState } from "react"
import { ChevronDownIcon, ChevronUpIcon } from "@primer/octicons-react"
import { Allotment, AllotmentHandle } from "allotment"
import Ansi from "ansi-to-react"
import * as api from "@/lib/api"
import { interdiff } from "@/lib/interdiff"
import { ThreeWayDiffBase, useThreeWayDiffBase } from "@/lib/settings"
import GhostButton from "../GhostButton"
@@ -26,23 +28,60 @@ export enum ProblemState {
ERRORS,
}
export type PerSaveObj = {
diff?: api.DiffOutput
}
export type Props = {
compilation: api.Compilation
isCompiling?: boolean
isCompilationOld?: boolean
selectedSourceLine: number | null
perSaveObj: PerSaveObj
}
export default function CompilationPanel({ compilation, isCompiling, isCompilationOld, selectedSourceLine }: Props) {
const [diff, setDiff] = useState<api.DiffOutput | null>(null)
export default function CompilationPanel({ compilation, isCompiling, isCompilationOld, selectedSourceLine, perSaveObj }: Props) {
const usedCompilationRef = useRef<api.Compilation | null>(null)
const problemState = getProblemState(compilation)
const [threeWayDiffBase] = useThreeWayDiffBase()
const [threeWayDiffEnabled, setThreeWayDiffEnabled] = useState(false)
const prevCompilation = usedCompilationRef.current
// Only update the diff if it's never been set or if the compilation succeeded
useEffect(() => {
if (!diff || compilation.success) {
setDiff(compilation.diff_output)
if (!usedCompilationRef.current || compilation.success) {
usedCompilationRef.current = compilation
}
const usedDiff = usedCompilationRef.current?.diff_output ?? null
// If this is the first time we re-render after a save, store the diff
// as a possible three-way diff base.
if (!perSaveObj.diff && usedCompilationRef.current?.success && usedDiff) {
perSaveObj.diff = usedDiff
}
const prevDiffRef = useRef<api.DiffOutput | null>(null)
let usedBase
if (threeWayDiffBase === ThreeWayDiffBase.SAVED) {
usedBase = perSaveObj.diff ?? null
prevDiffRef.current = null
} else {
if (compilation.success && compilation !== prevCompilation) {
prevDiffRef.current = prevCompilation?.diff_output ?? null
}
}, [compilation.diff_output, compilation.success, diff])
usedBase = prevDiffRef.current ?? null
}
const diff = useMemo(
() => {
if (threeWayDiffEnabled)
return interdiff(usedDiff, usedBase)
else
return usedDiff
},
[threeWayDiffEnabled, usedDiff, usedBase]
)
const container = useRef<HTMLDivElement>(null)
const allotment = useRef<AllotmentHandle>(null)
@@ -67,6 +106,9 @@ export default function CompilationPanel({ compilation, isCompiling, isCompilati
diff={diff}
isCompiling={isCompiling}
isCurrentOutdated={isCompilationOld || problemState == ProblemState.ERRORS}
threeWayDiffEnabled={threeWayDiffEnabled}
setThreeWayDiffEnabled={setThreeWayDiffEnabled}
threeWayDiffBase={threeWayDiffBase}
selectedSourceLine={selectedSourceLine}
/>
</Allotment.Pane>
@@ -32,6 +32,32 @@
user-select: none;
}
.spacer {
flex: 1;
}
.threeWayToggle {
position: relative;
padding: 4px 4px;
cursor: pointer;
&:hover {
border-radius: 4px;
color: var(--g1300);
background: var(--g400);
}
}
.threeWayToggleNumber {
position: absolute;
top: 27%;
left: 51%;
font-size: 0.7rem;
color: var(--g1500);
}
// Columns
.headers,
.row {
+57 -25
View File
@@ -1,14 +1,15 @@
/* eslint css-modules/no-unused-class: off */
import { createContext, CSSProperties, forwardRef, HTMLAttributes, Fragment, useContext, useEffect, useState } from "react"
import { createContext, CSSProperties, forwardRef, HTMLAttributes, Fragment, useContext, useRef, useState } from "react"
import { VersionsIcon } from "@primer/octicons-react"
import classNames from "classnames"
import AutoSizer from "react-virtualized-auto-sizer"
import { FixedSizeList } from "react-window"
import * as api from "@/lib/api"
import { useSize } from "@/lib/hooks"
import { useCodeFontSize } from "@/lib/settings"
import { ThreeWayDiffBase, useCodeFontSize } from "@/lib/settings"
import Loading from "../loading.svg"
@@ -141,7 +142,7 @@ const innerElementType = forwardRef<HTMLUListElement, HTMLAttributes<HTMLUListEl
})
innerElementType.displayName = "innerElementType"
function DiffBody({ diff, fontSize }: { diff: api.DiffOutput, fontSize: number | undefined }) {
function DiffBody({ diff, fontSize }: { diff: api.DiffOutput | null, fontSize: number | undefined }) {
const setHighlightAll: Highlighter["setValue"] = value => {
highlighter1.setValue(value)
highlighter2.setValue(value)
@@ -185,49 +186,78 @@ function DiffBody({ diff, fontSize }: { diff: api.DiffOutput, fontSize: number |
</div>
}
function ThreeWayToggleButton({ enabled, setEnabled }: { enabled: boolean, setEnabled: (enabled: boolean) => void }) {
return <button
className={styles.threeWayToggle}
onClick={() => {
setEnabled(!enabled)
}}
title={enabled ? "Disable three-way diffing" : "Enable three-way diffing"}
>
<VersionsIcon size={24} />
<div className={styles.threeWayToggleNumber}>
{enabled ? "3" : "2"}
</div>
</button>
}
export type Props = {
diff: api.DiffOutput
diff: api.DiffOutput | null
isCompiling: boolean
isCurrentOutdated: boolean
threeWayDiffEnabled: boolean
setThreeWayDiffEnabled: (value: boolean) => void
threeWayDiffBase: ThreeWayDiffBase
selectedSourceLine: number | null
}
export default function Diff({ diff, isCompiling, isCurrentOutdated, selectedSourceLine }: Props) {
export default function Diff({ diff, isCompiling, isCurrentOutdated, threeWayDiffEnabled, setThreeWayDiffEnabled, threeWayDiffBase, selectedSourceLine }: Props) {
const [fontSize] = useCodeFontSize()
const container = useSize<HTMLDivElement>()
const [barPos, setBarPos] = useState(NaN)
const [prevBarPos, setPrevBarPos] = useState(NaN)
const hasPreviousColumn = !!diff?.rows?.[0]?.previous
const [bar1Pos, setBar1Pos] = useState(NaN)
const [bar2Pos, setBar2Pos] = useState(NaN)
const columnMinWidth = 100
const clampedBarPos = Math.max(columnMinWidth, Math.min(container.width - columnMinWidth - (hasPreviousColumn ? columnMinWidth : 0), barPos))
const clampedPrevBarPos = hasPreviousColumn ? Math.max(clampedBarPos + columnMinWidth, Math.min(container.width - columnMinWidth, prevBarPos)) : container.width
const clampedBar1Pos = Math.max(columnMinWidth, Math.min(container.width - columnMinWidth - (threeWayDiffEnabled ? columnMinWidth : 0), bar1Pos))
const clampedBar2Pos = threeWayDiffEnabled ? Math.max(clampedBar1Pos + columnMinWidth, Math.min(container.width - columnMinWidth, bar2Pos)) : container.width
useEffect(() => {
// Distribute the bar positions across the container when its width changes
if (container.width) {
const numSections = hasPreviousColumn ? 3 : 2
// Distribute the bar positions across the container when its width changes
const updateBarPositions = (threeWayDiffEnabled: boolean) => {
const numSections = threeWayDiffEnabled ? 3 : 2
setBar1Pos(container.width / numSections)
setBar2Pos(container.width / numSections * 2)
}
const lastContainerWidthRef = useRef(NaN)
if (lastContainerWidthRef.current !== container.width && container.width) {
lastContainerWidthRef.current = container.width
updateBarPositions(threeWayDiffEnabled)
}
setBarPos(container.width / numSections)
setPrevBarPos(container.width / numSections * 2)
}
}, [container.width, hasPreviousColumn])
const threeWayButton = <>
<div className={styles.spacer} />
<ThreeWayToggleButton
enabled={threeWayDiffEnabled}
setEnabled={(enabled: boolean) => {
updateBarPositions(enabled)
setThreeWayDiffEnabled(enabled)
}}
/>
</>
return <div
ref={container.ref}
className={styles.diff}
style={{
"--diff-font-size": typeof fontSize == "number" ? `${fontSize}px` : "",
"--diff-left-width": `${clampedBarPos}px`,
"--diff-right-width": `${container.width - clampedPrevBarPos}px`,
"--diff-left-width": `${clampedBar1Pos}px`,
"--diff-right-width": `${container.width - clampedBar2Pos}px`,
"--diff-current-filter": isCurrentOutdated ? "grayscale(25%) brightness(70%)" : "",
} as CSSProperties}
>
<DragBar pos={clampedBarPos} onChange={setBarPos} />
{hasPreviousColumn && <DragBar pos={clampedPrevBarPos} onChange={setPrevBarPos} />}
<DragBar pos={clampedBar1Pos} onChange={setBar1Pos} />
{threeWayDiffEnabled && <DragBar pos={clampedBar2Pos} onChange={setBar2Pos} />}
<div className={styles.headers}>
<div className={styles.header}>
Target
@@ -235,9 +265,11 @@ export default function Diff({ diff, isCompiling, isCurrentOutdated, selectedSou
<div className={styles.header}>
Current
{isCompiling && <Loading width={20} height={20} />}
{!threeWayDiffEnabled && threeWayButton}
</div>
{hasPreviousColumn && <div className={styles.header}>
Previous
{threeWayDiffEnabled && <div className={styles.header}>
{threeWayDiffBase === ThreeWayDiffBase.SAVED ? "Saved" : "Previous"}
{threeWayButton}
</div>}
</div>
<SelectedSourceLineContext.Provider value={selectedSourceLine}>
+7 -1
View File
@@ -147,6 +147,10 @@ export default function Scratch({
onChange(scratch)
setIsModified(true)
}
const [perSaveObj, setPerSaveObj] = useState({})
const saveCallback = () => {
setPerSaveObj({})
}
const shouldCompare = !isModified
const sourceCompareExtension = useCompareExtension(sourceEditor, shouldCompare ? parentScratch?.source_code : undefined)
@@ -274,6 +278,7 @@ export default function Scratch({
isCompiling={isCompiling}
isCompilationOld={isCompilationOld}
selectedSourceLine={selectedSourceLine}
perSaveObj={perSaveObj}
/>}
</Tab>
case TabId.DECOMPILATION:
@@ -308,7 +313,7 @@ export default function Scratch({
const offlineOverlay = (
offline ? <>
<div className="fixed top-10 self-center rounded bg-red-8 px-3 py-2">
<p className="text-sm">The scratch editor is in offline mode. We're attempting to reconnect to the backend - as long as this tab is open, your work is safe.</p>
<p className="text-sm">The scratch editor is in offline mode. We're attempting to reconnect to the backend as long as this tab is open, your work is safe.</p>
</div>
</>
: <></>
@@ -326,6 +331,7 @@ export default function Scratch({
isCompiling={isCompiling}
scratch={scratch}
setScratch={setScratch}
saveCallback={saveCallback}
setDecompilationTabEnabled={setDecompilationTabEnabled}
/>
{matchProgressBarEnabledSetting && <div className={styles.progressbar}><ScratchProgressBar matchPercent={matchPercent}/></div>}
@@ -126,7 +126,7 @@ function ScratchName({ name, onChange }: { name: string, onChange?: (name: strin
}
}
function Actions({ isCompiling, compile, scratch, setScratch, setDecompilationTabEnabled }: Props) {
function Actions({ isCompiling, compile, scratch, setScratch, saveCallback, setDecompilationTabEnabled }: Props) {
const userIsYou = api.useUserIsYou()
const forkScratch = api.useForkScratchAndGo(scratch)
const [fuzzySaveAction, fuzzySaveScratch] = useFuzzySaveCallback(scratch, setScratch)
@@ -139,6 +139,7 @@ function Actions({ isCompiling, compile, scratch, setScratch, setDecompilationTa
setIsSaving(true)
await fuzzySaveScratch()
setIsSaving(false)
saveCallback()
})
const compileShortcut = useShortcut([SpecialKey.CTRL_COMMAND, "J"], () => {
@@ -161,6 +162,7 @@ function Actions({ isCompiling, compile, scratch, setScratch, setDecompilationTa
setIsSaving(true)
await fuzzySaveScratch()
setIsSaving(false)
saveCallback()
}}
disabled={!canSave || isSaving}
title={fuzzyShortcut}
@@ -171,7 +173,10 @@ function Actions({ isCompiling, compile, scratch, setScratch, setDecompilationTa
</li>
<li>
<button
onClick={forkScratch}
onClick={async () => {
await forkScratch()
saveCallback()
}}
title={fuzzySaveAction === FuzzySaveAction.FORK ? fuzzyShortcut : undefined}
>
<RepoForkedIcon />
@@ -250,6 +255,7 @@ export type Props = {
compile: () => Promise<void>
scratch: Readonly<api.Scratch>
setScratch: (scratch: Partial<api.Scratch>) => void
saveCallback: () => void
setDecompilationTabEnabled: (enabled: boolean) => void
}
+1 -1
View File
@@ -92,7 +92,7 @@ export function useSaveScratch(localScratch: Scratch): () => Promise<Scratch> {
libraries: undefinedIfUnchanged(savedScratch, localScratch, "libraries"),
})
await mutate(scratchUrl(localScratch), updatedScratch, false)
await mutate(scratchUrl(localScratch), updatedScratch, { revalidate: false })
return updatedScratch
}, [localScratch, savedScratch, userIsYou])
+108
View File
@@ -0,0 +1,108 @@
import { diff as myersDiff } from "fast-myers-diff"
import { DiffOutput, DiffRow } from "./api"
type Chunk = {
unaligned: DiffRow[]
aligned: DiffRow
}
function chunkDiffLines(rows: DiffRow[]): { chunks: Chunk[], lastUnaligned: DiffRow[] } {
let unaligned = []
const chunks = []
for (const row of rows) {
if (row.base) {
chunks.push({ unaligned, aligned: row })
unaligned = []
} else {
unaligned.push(row)
}
}
return { chunks, lastUnaligned: unaligned }
}
/**
* Combine two normal diffs into a three-way diff. This algorithm is derived from asm-differ:
* https://github.com/simonlindholm/asm-differ/blob/3841240be5e59c63f735f3cffc5a4c8dd16f14b1/diff.py#L3413-L3447
*/
export function interdiff(curr: DiffOutput | null, prev: DiffOutput | null): DiffOutput | null {
if (!curr || !prev)
return curr
const rows: DiffRow[] = []
const addMatching = (c: DiffRow, p: DiffRow) => {
if (c.key === p.key) {
rows.push(c)
} else {
rows.push({
key: c.key,
base: c.base,
current: c.current,
previous: p.current,
})
}
}
const addUnaligned = (cs: DiffRow[], ps: DiffRow[]) => {
if (!cs.length && !ps.length)
return
const ckeys = cs.map(c => c.key)
const pkeys = ps.map(p => p.key)
let ci = 0, pi = 0
// Array.from to silence an error about "Type 'IterableIterator<...>'
// can only be iterated through when using the '--downlevelIteration'
// flag or with a '--target' of 'es2015' or higher" -- changing
// tsconfig compilerOptions.target does not seem to work for me.
for (const [c0, c1, p0, p1] of Array.from(myersDiff(ckeys, pkeys))) {
if (c0 - ci !== p0 - pi) {
throw new Error("bad myers-diff range")
}
while (ci != c0)
addMatching(cs[ci++], ps[pi++])
while (ci != c1) {
const c = cs[ci++]
rows.push({
key: c.key,
current: c.current,
})
}
while (pi != p1) {
const p = ps[pi++]
rows.push({
key: p.key,
previous: p.current,
})
}
}
if (cs.length - ci !== ps.length - pi) {
throw new Error("bad myers-diff range")
}
while (ci != cs.length)
addMatching(cs[ci++], ps[pi++])
}
const currChunks = chunkDiffLines(curr.rows)
const prevChunks = chunkDiffLines(prev.rows)
if (currChunks.chunks.length !== prevChunks.chunks.length) {
// This should logically never happen, since the two diffs are based on
// the same target.
console.warn("Diff base changed size between two diffs?", curr, prev)
return curr
}
for (let i = 0; i < currChunks.chunks.length; i++) {
const c = currChunks.chunks[i]
const p = prevChunks.chunks[i]
addUnaligned(c.unaligned, p.unaligned)
addMatching(c.aligned, p.aligned)
}
addUnaligned(currChunks.lastUnaligned, prevChunks.lastUnaligned)
return {
arch_str: curr.arch_str,
current_score: curr.current_score,
max_score: curr.max_score,
header: curr.header,
rows: rows,
}
return curr
}
+7
View File
@@ -14,6 +14,12 @@ const codeColorScheme = createPersistedState<ColorScheme>("codeColorScheme")
const languageServerEnabled = createPersistedState<boolean>("languageServerEnabled")
const matchProgressBarEnabled = createPersistedState<boolean>("matchProgressBarEnabled")
const vimModeEnabled = createPersistedState<boolean>("vimModeEnabled")
const threeWayDiffBase = createPersistedState<ThreeWayDiffBase>("threeWayDiffBase")
export enum ThreeWayDiffBase {
SAVED = "saved",
PREV = "prev",
}
export const useTheme = () => theme("auto")
export const useAutoRecompileSetting = () => autoRecompile(true)
@@ -25,6 +31,7 @@ export const useCodeColorScheme = () => codeColorScheme("Frog Dark")
export const useLanguageServerEnabled = () => languageServerEnabled(false)
export const useMatchProgressBarEnabled = () => matchProgressBarEnabled(true)
export const useVimModeEnabled = () => vimModeEnabled(false)
export const useThreeWayDiffBase = () => threeWayDiffBase(ThreeWayDiffBase.SAVED)
export function useIsSiteThemeDark() {
const [theme] = useTheme()