Closed grishat closed 2 years ago
Hey @grishat
Thanks for this. Ill take more time to go over the code as soon as possible. That said, ill share my initial thoughts here.
My original plan was to IgnoreChildrenElementComparer
that looks something like this:
public static class IgnoreChildrenElementComparer
{
private const string DIFF_IGNORE_CHILDREN_ATTRIBUTE = "diff:ignorechildren";
/// <summary>
/// The ignore children element comparer.
/// </summary>
public static CompareResult Compare(in Comparison comparison, CompareResult currentDecision)
{
if (currentDecision == CompareResult.Skip)
return currentDecision;
return ParentControlHasTruthyIgnoreChildrenAttribute(comparison)
? currentDecision | CompareResult.Skip
: currentDecision;
}
private static bool ParentControlHasTruthyIgnoreChildrenAttribute(in Comparison comparison)
{
return comparison.Control.Node is IElement element &&
element.Parent is IElement parentElement &&
parentElement.TryGetAttrValue(DIFF_IGNORE_CHILDREN_ATTRIBUTE, out bool shouldIgnore) &&
shouldIgnore;
}
}
Did you try something like that? I think it should work, and then it will not require adding additional options to the CompareResult
enum, which I prefer.
Hi @egil!
At first I tried to do this like in your code. I don't like the changes to the CompareResult
enum too.
But it doesn't work in this case: <p diff:ignoreChildren></p>
compare to <p><em></em></p>
. That is, the child element is in the test and not in the control.
I also tried to extend CompareResult (like tuple (CompareResult result, bool ignoreChildren)), because CompareResult may be Same (or Different) and at the same time SkipChildren. But it requires too many changes.
Hi @egil!
At first I tried to do this like in your code. I don't like the changes to the
CompareResult
enum too. But it doesn't work in this case:<p diff:ignoreChildren></p>
compare to<p><em></em></p>
. That is, the child element is in the test and not in the control. I also tried to extend CompareResult (like tuple (CompareResult result, bool ignoreChildren)), because CompareResult may be Same (or Different) and at the same time SkipChildren. But it requires too many changes.
In that case I like your solution with turning the CompareResult
into a flags enum.
@egil, I had an idea to do like this:
private static bool ControlParentHasTruthyIgnoreAttribute(in Comparison comparison)
{
var resultControl = comparison.Control.Node is IElement element &&
element.ParentElement is not null &&
element.ParentElement.TryGetAttrValue(DIFF_IGNORE_CHILDREN_ATTRIBUTE, out bool shouldIgnore) &&
shouldIgnore;
var resultTest = comparison.Test.Node is IElement testElement &&
element.ParentElement is not null &&
element.ParentElement.TryGetAttrValue(DIFF_IGNORE_CHILDREN_ATTRIBUTE, out bool shouldIgnore) &&
shouldIgnore;
return resultControl | resultTest;
}
I don't understand how to take a parent of the Control element by testElement. I'm not sure, but I'll try it tomorrow.
Also. It would be awesome if you could add the
diff:ignoreAttributes
feature as well. That indiff:ignoreChildren
kinda goes hand in hand :)
Ok
@egil, I had an idea to do like this:
private static bool ControlParentHasTruthyIgnoreAttribute(in Comparison comparison) { var resultControl = comparison.Control.Node is IElement element && element.ParentElement is not null && element.ParentElement.TryGetAttrValue(DIFF_IGNORE_CHILDREN_ATTRIBUTE, out bool shouldIgnore) && shouldIgnore; var resultTest = comparison.Test.Node is IElement testElement && element.ParentElement is not null && element.ParentElement.TryGetAttrValue(DIFF_IGNORE_CHILDREN_ATTRIBUTE, out bool shouldIgnore) && shouldIgnore; return resultControl | resultTest; }
I don't understand how to take a parent of the Control element by testElement. I'm not sure, but I'll try it tomorrow.
Maybe you can add a custom matcher that matches a diff:ignoreChildren element with all child elements in the test nodes tree. Or maybe an custom filter that that simply removes child nodes from the comparison.
I admit its been almost two years since I looked deeply at this, so im a bit rusty :)
Hi @egil
I also added diff:ignoreAttributes element comparer. I think, using 'CompareResult' as flags enum is the best solution for now: small changes and no performance reduction.
Could you add after this PR changes to bUnit? Or I must open an issue on bUnit and create new PR?
Hi @egil
I also added diff:ignoreAttributes element comparer.
Great, thanks. I'll take a look later.
I think, using 'CompareResult' as flags enum is the best solution for now: small changes and no performance reduction.
Agreed.
Could you add after this PR changes to bUnit? Or I must open an issue on bUnit and create new PR?
I'll update bunit's anglesharp.diffing dependencies when this is merged and released.
Could you add after this PR changes to bUnit? Or I must open an issue on bUnit and create new PR?
When I have pushed a new verison diffing, I think you can just include the new version explicitly in your bUnit test project and it should be used instead of the version referenced by bUnit.
Thank you very much for this!
Types of Changes
Added 'diff:ignoreChildren' comparer. Now there is the ability to ignore an elements children (see #9).
Prerequisites
Please make sure you can check the following two boxes:
Contribution Type
What types of changes does your code introduce? Put an
x
in all the boxes that apply:Description
I can update documentation after accepting PR.