-
-
Notifications
You must be signed in to change notification settings - Fork 43
JSON normalization followup #241
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This pull request implements JSON normalization features and refactors diff options handling alongside some UI tweaks. Key changes include renaming CSS classes for diff mode controls, consolidating diff options parsing/encoding in TypeScript, and adding the keyboard shortcut z for JSON normalization.
Reviewed Changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
webdiff/static/css/style.css | Renamed CSS classes from image-diff-controls to diff-mode-controls and added styling for warning banner. |
ts/utils.ts | Added a new Unionize type. |
ts/options.ts | Refactored options logic to merge GitDiffOptions into a unified Options interface. |
ts/diff-options.ts | Renamed and updated encoding/decoding functions to operate on GitDiffOptions. |
ts/codediff/codediff.tsx | Introduced a safety check for large diffs with a bypass mechanism. |
ts/codediff/NormalizeJSONOption.tsx | Updated normalization option component with new class names and label text. |
ts/DiffView.tsx, ts/DiffOptions.tsx, ts/CodeDiffContainer.tsx, ts/Root.tsx | Updated diff options usages and renamed functions to align with new options workflow. |
ts/ImageDiff.tsx | Updated image diff controls to use the new diff-mode-controls class and added an explicit property for normalization equality. |
ts/tests/diff-options.test.ts | Updated tests to use the new diff options functions. |
Other files | Version bump and minor adjustments in test data and project config. |
Comments suppressed due to low confidence (1)
ts/Root.tsx:63
- Consider using a functional update (e.g., setDiffOptions(prev => ({...prev, ...update(prev)}))) to merge option changes, which can help prevent potential stale closure issues when options are updated asynchronously.
const updateOptions = React.useCallback<UpdateOptionsFn>( update => { setDiffOptions({...options, ...(typeof update === 'function' ? update(options) : update)}); }, [options, setDiffOptions]);
Deals with some of the tasks in #239.
Also refactors some of the options code.
Warning banner if the first line is longer than 200k:
Message if the content is identical post-normalization:
I went with z as the shortcut for normalization.