-
-
Notifications
You must be signed in to change notification settings - Fork 193
Added new Diff on both DiffBuilders for more detailed DiffModel. #83
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,10 +1,13 @@ | ||
using System.Collections.Generic; | ||
using System; | ||
using System.Collections.Generic; | ||
using System.Linq; | ||
|
||
namespace DiffPlex.DiffBuilder.Model | ||
{ | ||
public class DiffPaneModel | ||
{ | ||
public List<DiffPiece> Chunks => Lines; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why make an alias like this? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because when Line chunker isn't used as first the current name "Lines" doesn't represent its contents. But its just a small alias np for me to delete it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sounds fine, can you put summary comments on both maybe to clear up any confusion? |
||
|
||
public List<DiffPiece> Lines { get; } | ||
|
||
public bool HasDifferences | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -104,6 +104,59 @@ public static SideBySideDiffModel Diff(IDiffer differ, string oldText, string ne | |
return model; | ||
} | ||
|
||
public static SideBySideDiffModel Diff( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Couldn't we update the other methods in this file to use the new more general method here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point If unit tests will still pass I'll look into this. |
||
IDiffer differ, | ||
string oldText, string newText, | ||
List<IChunker> detailsPack, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I like the idea to give an array, but detailsPack name is too general. maybe called it chunkers or something There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oka then Chunkers it is. |
||
bool ignoreWhiteSpace = true, bool ignoreCase = false) | ||
{ | ||
if (oldText == null) throw new ArgumentNullException(nameof(oldText)); | ||
if (newText == null) throw new ArgumentNullException(nameof(newText)); | ||
|
||
if (differ == null) return Diff(oldText, newText, ignoreWhiteSpace, ignoreCase); | ||
|
||
LinkedList<IChunker> chunkers; | ||
if (detailsPack == null || !detailsPack.Any()) | ||
{ | ||
chunkers = new LinkedList<IChunker>(); | ||
chunkers.AddLast(DiffPlex.Chunkers.LineChunker.Instance); | ||
chunkers.AddLast(DiffPlex.Chunkers.WordChunker.Instance); | ||
chunkers.AddLast(DiffPlex.Chunkers.CharacterChunker.Instance); | ||
} | ||
else | ||
{ | ||
chunkers = new LinkedList<IChunker>(detailsPack); | ||
} | ||
|
||
var model = new SideBySideDiffModel(); | ||
var cnode = chunkers.First; | ||
|
||
var diffResult = differ.CreateDiffs(oldText, newText, ignoreWhiteSpace, ignoreCase, cnode.Value); | ||
BuildDiffPieces(diffResult, model.OldText.Lines, model.NewText.Lines, NextPieceBuilderInternal(differ, cnode.Next), ignoreWhiteSpace, ignoreCase); | ||
|
||
return model; | ||
} | ||
|
||
|
||
private static PieceBuilder NextPieceBuilderInternal( | ||
IDiffer differ, | ||
LinkedListNode<IChunker> chunkerNode) | ||
{ | ||
if (chunkerNode == null) | ||
{ | ||
return null; | ||
} | ||
else | ||
{ | ||
return (ot, nt, op, np, iw, ic) => | ||
{ | ||
var r = differ.CreateDiffs(ot, nt, iw, ic, chunkerNode.Value); | ||
return BuildDiffPieces(r, op, np, NextPieceBuilderInternal(differ, chunkerNode.Next), iw, ic); | ||
}; | ||
} | ||
} | ||
|
||
|
||
private static ChangeType BuildWordDiffPiecesInternal(string oldText, string newText, List<DiffPiece> oldPieces, List<DiffPiece> newPieces, bool ignoreWhiteSpace, bool ignoreCase) | ||
{ | ||
var diffResult = Differ.Instance.CreateDiffs(oldText, newText, ignoreWhiteSpace, ignoreCase, WordChunker.Instance); | ||
|
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.
Instread of having this logic close to twice, could we instread just share the
BuildDiffPieces
logic that SideBySide uses, extract it to a common place? Then let inline do one last step to flatten it?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.
Yes you are probably right. Was trying to solve it twice now.