HexEditor diff #522

tomilho · 2024-05-16T17:02:47Z

MVP to receive some early feedback on the diff process. (Closes #47 )

As I was unsure if it was possible to do this since there isn't any explicit API to create custom diff, I rushed most components to figure out the diff process. There is still plenty of work to do, but hopefully this shows how the diff can look.

Simple Diff Algorithm

Improved Diff Alg

There are some issues due to VSCode not supporting custom diff editors (I think), below is a list.

(Unfixable?) Issues

Switching Tabs - vscode.window.TabGroup does not have a class type for custom editor's diff, so .input is undefined.
No Status Bar Item display
Keybinds do not work - Insert does not work

Applies custom css styles on data cells by specifying a decorator style and its offset range.

Opens two selected files into hex diff view. Works with any file extension by changing the file scheme.

Introduces the O(d^2) space version and its decorators. With this diff algorithm, the diffs should be more insightful compared to using a simple per-offset comparison.

Inserted bytes are show in the original file by a stripe data cell.

tomilho · 2024-07-03T18:42:18Z

I changed the diff algorithm, lmk what you think.

Finds the lower and upper bound of the decorators that fit in the page, instead of using .filter(). In addition to that, it also removes the need to re-initialize the decorator's ranges.

connor4312 · 2024-07-08T22:21:49Z

Thanks for the PR, looks super cool! I'll give this a look this week.

You're right in that there's no "proper" support for custom diff editors in VS Code. That's something I would like to get done in the future, but this is a nice start and should be reusable if/when support arrives.

lramos15 · 2024-07-08T22:23:07Z

This is really neat. I've added it to my review queue when I get some free time.

cc @mjbvz if you have any feedback on the best way to do custom editor diffs since it's not very common there are not many examples of this

connor4312 · 2024-07-08T22:50:06Z

also, regarding diff algorithms, I wonder if we can use existing implementations. I'm sure there are good libraries out there, though maybe ones that would need to be compiled to wasm. Doesn't need to happen in this PR.

tomilho · 2024-07-09T17:57:15Z

Thank you for the comments!! I will for sure look into some libraries since I am not happy at all with my latest version (comparing 2MB files results in a crash). But please don't feel pressured to prioritize this as it is a WIP.

tomilho · 2024-07-19T11:59:12Z

Regarding diff algorithms, I really liked the wfa2-lib diff used in biodiff. However, it does not work properly in wasm32 as it is coded to work for 64bit but since wasm64 is right around the corner, I think we can use diff in the meantime. And when wasm64 is released, I can bring wfa2 into the project.

Let me know if this is okay with you.

connor4312 · 2024-07-19T16:39:38Z

Another thing we could do is make the hex editor a platform-specific extension and ship binaries that are subprocessed into on supported platforms (64 bit platforms minus web, presumably.) This is not super hard to do from an engineering perspective since Python recently paved the way with its native locator. I can try and carve out some time next iteration to get that in if you think it makes sense :)

We could use the JS diff library on platforms where that's not supported. We may want to do that on a worker_thread though to avoid blocking the extension host on very large diffs.

If you want to get that going in this PR I can follow up next month with a wfa2/biodiff version.

tomilho · 2024-07-19T19:17:19Z

I had no idea vscode extensions could ship with binaries! I really like the idea to be honest, especially when wasm64 may still take some time even though there is only one task left. I will start with the JS diff and worker_thread. Let me know if I can help you with the wfa2 stuff.

Done to simplify diff, because recomputing diffs after a file change is too complex to tackle at the moment.

Although still O(d^2), it improves memory usage in other ways, provides some optimization for edge cases and has some useful built-in options to exit on large diffs.

tomilho · 2024-07-24T16:11:17Z

I've added the worker and the JS Diff. I did the worker such that it only works when running web vscode, but I wasn't entirely sure how to structure it, see these lines.

connor4312 · 2024-07-24T17:17:38Z

You can also do it in Node.js using the worker_threads module which implements a very similar interface to the webworker :)

connor4312 · 2024-07-25T20:03:29Z

Cool, thanks! I'll give this a more thorough review soon.

I also put in an issue for biodiff for avenues we could explore to leverage it here: 8051Enthusiast/biodiff#20

tomilho · 2024-07-25T22:07:51Z

Thank you! Let me know how I can help

connor4312

Nice work as usual, some comments :)

media/editor/dataDisplay.tsx

connor4312 · 2024-07-25T21:19:17Z

src/extension.ts

+	let worker: Worker;
+	const workerFilePath = vscode.Uri.joinPath(
+		context.extensionUri,
+		"dist",
+		"diffWorker.js",
+	).toString();
+
+	try {
+		worker = new Worker(workerFilePath);
+	} catch {
+		// eslint-disable-next-line @typescript-eslint/no-var-requires
+		const { Worker } = require("worker_threads") as typeof import("worker_threads");
+		const nodeWorker = new Worker(new URL(workerFilePath));
+		// Web and node js have different worker interfaces, so we share a function
+		// to initialize both workers the same way.
+		const ref = nodeWorker.addListener;
+		(nodeWorker as any).addEventListener = ref;
+		worker = nodeWorker as any;
+	}
+
+	const workerMessageHandler = new MessageHandler<ToDiffWorkerMessage, FromDiffWorkerMessage>(
+		// Always return undefined as the diff worker
+		// does not request anything from extension host
+		async () => undefined,
+		message => worker.postMessage(message),
+	);
+
+	worker.addEventListener("message", e =>
+		// e.data is used in web worker and e is used in node js worker
+		e.data ? workerMessageHandler.handleMessage(e.data) : workerMessageHandler.handleMessage(e as any),
+	);


I think this code could be moved into its own file and the Worker instantiated lazily rather than at activation time. Most usages of the hex editor won't be using it for diff operations, so this may be wasted work.

I have done both, but I still kept the dispose to be done when the extension is deactivated since I am not entirely sure when to dispose it. Should I keep it like this?

src/extension.ts

shared/hexDiffModel.ts

connor4312 · 2024-07-25T22:26:10Z

src/hexEditorRegistry.ts

@@ -68,6 +70,20 @@ export class HexEditorRegistry extends Disposable {
 		};
 	}

+	/** adds a diff model */
+	public addDiff(diffModelBuilder: HexDiffModelBuilder) {


The lifecycle of this is kind of weird and I'm not sure all cases are handled, such as reopening an editor without re-running the "compare selected". It seems like the main point of this is sharing the model builder between the two URIs.

Instead I would suggest stashing some query parameters when you build the URIs like ?token={uuid}&side={modified|original} having the getDiff be something like

getDiff(uri: vscode.Uri): { builder: HexDiffModelBuilder, dispose: () => void } { const { token, side } = parseQueryStringFrom(uri); if (!this.diffsBuilder.get(token)) { this.diffsBuilder.set(token, { refCount: 0, value: new DiffsBuilder() }); } const builder = this.diffsBuilder.get(token); builder.refCount++; builder.value.set(side, uri); return { builder: builder.value, dispose: () => { /* refCount--, remove if 0 */ }} }

...and have HexDiffModelBuilder.build() await both sides being set similarly to how it works for the documents today. That approach should allow things to work and the editor to be reopened/moved/duplicated without issue.

Thanks for the great suggestion, it made the lifecycle a lot simpler! I did some small tweaks, but overall it should be the same.

tomilho · 2024-08-04T09:03:34Z

Thank you for the review and sorry for the late changes.

connor4312

Lgtm, nice work!

tomilho · 2024-08-10T09:21:40Z

Thank you for the opportunity to work on this and taking your time to review everything! I learnt so much about diffing and vscode.

heartacker · 2024-09-06T00:23:23Z

is this release

tomilho added 6 commits July 2, 2024 23:46

feat: simple decorators

da6ff2b

Applies custom css styles on data cells by specifying a decorator style and its offset range.

refactor: move parseQuery to its own util file

5c0cdbb

feat: compareSelected command

d8d31e5

Opens two selected files into hex diff view. Works with any file extension by changing the file scheme.

synchronizes hex documents creation for diffing

41428e6

feat: basic Myers diff

393d697

Introduces the O(d^2) space version and its decorators. With this diff algorithm, the diffs should be more insightful compared to using a simple per-offset comparison.

Adds alignment in original file

6312f63

Inserted bytes are show in the original file by a stripe data cell.

tomilho force-pushed the fr-diff branch from cb68131 to 6312f63 Compare July 3, 2024 18:29

tomilho added 2 commits July 4, 2024 20:17

fix: negative k-index and wrong alignment

8013725

optimize: use binary search for page decorators

eb76601

Finds the lower and upper bound of the decorators that fit in the page, instead of using .filter(). In addition to that, it also removes the need to re-initialize the decorator's ranges.

tomilho added 4 commits July 19, 2024 20:46

Makes diff read-only

821aa5d

Done to simplify diff, because recomputing diffs after a file change is too complex to tackle at the moment.

refactor: use diff package for myers diff

954158d

Although still O(d^2), it improves memory usage in other ways, provides some optimization for edge cases and has some useful built-in options to exit on large diffs.

fix: diff failing to open in web

f721459

perf: moved myers diff into a web-only worker

5616364

adds node js worker

2783303

tomilho marked this pull request as ready for review July 25, 2024 22:07

connor4312 reviewed Jul 25, 2024

View reviewed changes

tomilho added 2 commits July 29, 2024 16:21

adds worker transferable objects

6f54c8c

Use binary search in data row content

970ad7b

tomilho added 3 commits August 3, 2024 21:16

removes web-only code

d8befa2

fix: decorator's binary search gives wrong index

ce668a1

Adds lazy init & better diff lifecycle

f03b842

connor4312 approved these changes Aug 9, 2024

View reviewed changes

connor4312 enabled auto-merge (squash) August 9, 2024 23:04

vs-code-engineering bot added this to the August 2024 milestone Aug 9, 2024

bpasero approved these changes Aug 10, 2024

View reviewed changes

connor4312 merged commit dce2189 into microsoft:main Aug 10, 2024
3 checks passed

tomilho deleted the fr-diff branch August 12, 2024 21:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

HexEditor diff #522

HexEditor diff #522

tomilho commented May 16, 2024 •

edited

Loading

tomilho commented Jul 3, 2024

connor4312 commented Jul 8, 2024

lramos15 commented Jul 8, 2024

connor4312 commented Jul 8, 2024 •

edited

Loading

tomilho commented Jul 9, 2024

tomilho commented Jul 19, 2024

connor4312 commented Jul 19, 2024 •

edited

Loading

tomilho commented Jul 19, 2024

tomilho commented Jul 24, 2024

connor4312 commented Jul 24, 2024

connor4312 commented Jul 25, 2024

tomilho commented Jul 25, 2024

connor4312 left a comment

connor4312 Jul 25, 2024

tomilho Aug 4, 2024

connor4312 Jul 25, 2024

tomilho Aug 4, 2024

tomilho commented Aug 4, 2024

connor4312 left a comment

tomilho commented Aug 10, 2024

heartacker commented Sep 6, 2024

HexEditor diff #522

HexEditor diff #522

Conversation

tomilho commented May 16, 2024 • edited Loading

Simple Diff Algorithm

Improved Diff Alg

(Unfixable?) Issues

tomilho commented Jul 3, 2024

connor4312 commented Jul 8, 2024

lramos15 commented Jul 8, 2024

connor4312 commented Jul 8, 2024 • edited Loading

tomilho commented Jul 9, 2024

tomilho commented Jul 19, 2024

connor4312 commented Jul 19, 2024 • edited Loading

tomilho commented Jul 19, 2024

tomilho commented Jul 24, 2024

connor4312 commented Jul 24, 2024

connor4312 commented Jul 25, 2024

tomilho commented Jul 25, 2024

connor4312 left a comment

Choose a reason for hiding this comment

connor4312 Jul 25, 2024

Choose a reason for hiding this comment

tomilho Aug 4, 2024

Choose a reason for hiding this comment

connor4312 Jul 25, 2024

Choose a reason for hiding this comment

tomilho Aug 4, 2024

Choose a reason for hiding this comment

tomilho commented Aug 4, 2024

connor4312 left a comment

Choose a reason for hiding this comment

tomilho commented Aug 10, 2024

heartacker commented Sep 6, 2024

tomilho commented May 16, 2024 •

edited

Loading

connor4312 commented Jul 8, 2024 •

edited

Loading

connor4312 commented Jul 19, 2024 •

edited

Loading