Skip to content

Add char diff option #31

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

Add char diff option #31

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ class Diff extends PureComponent {
|newValue |`string` |`''` |New value as string. |
|splitView |`boolean` |`true` |Switch between `unified` and `split` view. |
|disableWordDiff |`boolean` |`false` |Show and hide word diff in a diff line. |
|useCharDiff |`boolean` |`true` |Use char-based diff by default. |
Copy link
Owner

@praneshr praneshr Sep 15, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

useCharDiff sounds confusing. What will happen when disableWordDiff: true and useCharDiff: true?

I would prefer to have one prop to control this. Say, diffType and it can take 3 possible values.

diffType: line|lineAndWord|lineAndChar

line: It will be the default behaviour. It shows the line-wise diff.
lineAndWord: It will show line and word-wise diff.
lineAndChar: It will show line and char-wise diff.

These three values can also be an enum. I prefer this over the previous method. For example,

export enum DiffType {
	LINE = 0,
	LINE_AND_WORD = 1,
	LINE_AND_CHAR = 2,
}
import React, { PureComponent } from 'react'
import ReactDiffViewer, {DiffType} from 'react-diff-viewer'

class Diff extends PureComponent {
  render = () => {
    return (
      <ReactDiffViewer
		diffType={DiffType.LINE_AND_CHAR}
        oldValue={oldCode}
        newValue={newCode}
        splitView={true}
      />
    )
  }
}

Suggestions are welcome

Copy link
Author

@bstst bstst Sep 16, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer this approach too.

But I'm now somewhat confused about the defaults. Right now disableWordDiff is false by default. Which means that the diff is computed by using diff.diffChars by default. If I enable disableWordDiff — then neither diff.diffChars nor diff.diffWords is used, and the whole line is diffed. Which kind of implies that we need to enum this instead:

export enum DiffType {
	LINE = 0,
	WORD = 1,
        CHAR = 2,
}

Is my assumption correct?

|hideLineNumbers |`boolean` |`false` |Show and hide line numbers. |
|renderContent |`function` |`undefined` |Render Prop API to render code in the diff viewer. Helpful for [syntax highlighting](#syntax-highlighting) |
|onLineNumberClick |`function` |`undefined` |Event handler for line number click. `(lineId: string) => void` |
Expand Down
29 changes: 29 additions & 0 deletions examples/src/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ interface ExampleState {
highlightLine?: string[];
language?: string;
enableSyntaxHighlighting?: boolean;
useCharDiff?: boolean;
}

const P = (window as any).Prism;
Expand All @@ -31,6 +32,7 @@ class Example extends React.Component<{}, ExampleState> {
highlightLine: [],
language: 'javascript',
enableSyntaxHighlighting: true,
useCharDiff: true,
};
}

Expand Down Expand Up @@ -200,6 +202,33 @@ class Example extends React.Component<{}, ExampleState> {
}
/>
</div>
<div className="controls">
<span>Char / word diffs:</span>
<span>
<label>
<input
type="checkbox"
name="toggle-2"
id="toggle-2"
onChange={(e) => this.setState({ useCharDiff: e.target.checked })}
checked={this.state.useCharDiff}
/>{' '}
Use char diff
</label>
</span>
</div>
<div className="diff-viewer">
<ReactDiff
highlightLines={this.state.highlightLine}
splitView={true}
useCharDiff={this.state.useCharDiff}
oldValue="This is a test"
newValue="That is a toast"
renderContent={
this.state.enableSyntaxHighlighting && this.syntaxHighlight
}
/>
</div>
<footer>
Made with 💓 by{' '}
<a href="https://praneshravi.in" target="_blank">
Expand Down
12 changes: 7 additions & 5 deletions src/compute-lines.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,9 @@ const constructLines = (value: string): string[] => {
* @param oldValue Old word in the line.
* @param newValue New word in the line.
*/
const computeWordDiff = (oldValue: string, newValue: string): WordDiffInformation => {
const computeDiff = (oldValue: string, newValue: string, useCharDiff: boolean): WordDiffInformation => {
const diffArray = diff
.diffChars(oldValue, newValue);
[useCharDiff ? 'diffChars' : 'diffWords'](oldValue, newValue);
Copy link
Owner

@praneshr praneshr Sep 15, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can be replaced with the following for better readability.

const diffArray = useCharDiff ? diff.diffChars(oldValue, newValue) : diff.diffChars(oldValue, newValue)

const wordDiff: WordDiffInformation = {
left: [],
right: [],
Expand Down Expand Up @@ -106,11 +106,13 @@ const computeWordDiff = (oldValue: string, newValue: string): WordDiffInformatio
* @param oldString Old string to compare.
* @param newString New string to compare with old string.
* @param disableWordDiff Flag to enable/disable word diff.
* @param useCharDiff Flag to use.
*/
const computeLineInformation = (
oldString: string,
newString: string,
disableWordDiff: boolean = false,
useCharDiff: boolean = true,
): ComputedLineInformation => {
const diffArray = diff.diffLines(
oldString.trimRight(),
Expand Down Expand Up @@ -172,9 +174,9 @@ const computeLineInformation = (
if (disableWordDiff) {
right.value = rightValue;
} else {
const wordDiff = computeWordDiff(line, rightValue as string);
right.value = wordDiff.right;
left.value = wordDiff.left;
const computedDiff = computeDiff(line, rightValue as string, useCharDiff);
right.value = computedDiff.right;
left.value = computedDiff.left;
}
}
} else {
Expand Down
4 changes: 4 additions & 0 deletions src/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ export interface ReactDiffViewerProps {
highlightLines?: string[];
// Style overrides.
styles?: ReactDiffViewerStylesOverride;
// Use char or word diff, true by default.
useCharDiff?: boolean;
}

export interface ReactDiffViewerState {
Expand All @@ -72,6 +74,7 @@ class DiffViewer extends React.Component<ReactDiffViewerProps, ReactDiffViewerSt
hideLineNumbers: false,
extraLinesSurroundingDiff: 3,
showDiffOnly: true,
useCharDiff: true,
Copy link
Owner

@praneshr praneshr Sep 15, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The default value should be false else it will break the existing behaviour.

Copy link
Author

@bstst bstst Sep 16, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At the moment compute-lines.ts is doing diff.diffChars(oldValue, newValue), which is the same as having true as default value for this option, or am I confusing something?

};

public static propTypes = {
Expand Down Expand Up @@ -425,6 +428,7 @@ class DiffViewer extends React.Component<ReactDiffViewerProps, ReactDiffViewerSt
oldValue,
newValue,
this.props.disableWordDiff,
this.props.useCharDiff,
);
const extraLines = this.props.extraLinesSurroundingDiff < 0
? 0
Expand Down
66 changes: 52 additions & 14 deletions test/compute-lines-test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,13 +135,11 @@ describe('Testing compute lines utils', (): void => {
});
});

it('Should identify word diff', (): void => {
const oldCode = `test
oldLine`;
const newCode = `test
newLine`;
it('Should use a char diff', (): void => {
const oldCode = `test\noldLine`;
const newCode = `test\nnewLine`;

expect(computeLineInformation(oldCode, newCode))
expect(computeLineInformation(oldCode, newCode, false, true))
.toMatchObject({
lineInformation: [
{
Expand All @@ -161,10 +159,6 @@ describe('Testing compute lines utils', (): void => {
lineNumber: 2,
type: 1,
value: [
{
type: 0,
value: ' ',
},
{
type: 1,
value: 'new',
Expand All @@ -179,10 +173,6 @@ describe('Testing compute lines utils', (): void => {
lineNumber: 2,
type: 2,
value: [
{
type: 0,
value: ' ',
},
{
type: 2,
value: 'old',
Expand All @@ -198,4 +188,52 @@ describe('Testing compute lines utils', (): void => {
diffLines: [1],
});
});

it('Should use a word diff', (): void => {
const oldCode = `test\noldLine`;
const newCode = `test\nnewLine`;

expect(computeLineInformation(oldCode, newCode, false, false))
.toMatchObject({
"diffLines": [
1,
],
"lineInformation": [
{
"left": {
"lineNumber": 1,
"type": 0,
"value": "test",
},
"right": {
"lineNumber": 1,
"type": 0,
"value": "test",
},
},
{
"left": {
"lineNumber": 2,
"type": 2,
"value": [
{
"type": 2,
"value": "oldLine",
},
],
},
"right": {
"lineNumber": 2,
"type": 1,
"value": [
{
"type": 1,
"value": "newLine",
},
],
},
},
],
});
});
});