search

xspec / oXygen-XML-editor-xspec-support

An oxygen XML Editor plugin intended to help those that create XSpec scenarios.
MIT License
11 stars 10 forks source link

Diff action shows a "/" in first line #29

Closed tofi86 closed 5 years ago

tofi86 commented 6 years ago

Testcase: https://github.com/xspec/xspec/issues/227

The test fails for whatever reasons will come up in the named issue.

However, when clicking the "Diff" button, the oXygen Diff shows a / character in the first line of "expected", which isn't contained in the expected file:

bildschirmfoto 2018-07-24 um 12 54 01

tofi86 commented 6 years ago

Also, the <test:ws> elements shouldn't be visible, right?

AlexJitianu commented 6 years ago

Also, the elements shouldn't be visible, right?

I'm not sure what to do with them... They look like they contain the original spacing/indent but if that's the case I would have expected them to appear for each indent. If I look inside the intermediate *-result.xml file I see one such test:ws element between each two tags from the x:expect element, but in the x:result one there are text:ws elements missing (for example between the and element).

do you have any suggestions? Should I remove, unwrap or keep them as the only indent (and discard the other indent nodes).

tofi86 commented 6 years ago

I'm not sure what to do with them... They look like they contain the original spacing/indent

That's what it is indeed: https://github.com/xspec/xspec/issues/227#issuecomment-407469485 But I also had to learn it in first place...

So what to do? I'm undecided... From a technical point I understand why it's important to have this information around, but from a UX perspective I was (and still am) a bit confused to see it...

Removing all other textnodes and unwrapping the original whitespace nodes seems like a nice idea...

@cirulls @AirQuick do you have an idea? Or should @AlexJitianu just leave the diff as is? I would be fine with it now that I know what the test:ws elements are for... but I could also image the solution mentioned above...

AirQuick commented 6 years ago

The whitespace-only text node in test:ws is significant. So it shouldn't be removed.

We could just unwrap it. But the screenshot suggests that the problem happens on oXygen Diff, right? Then I recall that oXygen Diff has Format and Indent Both Files button (which itself is quite useful). If the user hits the button, I'm afraid the unwrapped whitespace-only text node is lost.

Another option might be unwrapping test:ws and replacing its whitespace-only text node with a string just like the HTML report does. In this case, the user may still wonder what this \n\r\t. string is... But I guess at least it isn't too ugly nor lost by Format and Indent.

neeraj0015 commented 5 years ago

Hi, For ignoring to you have to remove extra line and remove new line your input[wrap input element].

neeraj0015 commented 5 years ago

Also, the <test:ws> elements shouldn't be visible, right?

Hi, For ignoring to test:ws you have to remove extra line and remove new line your input[wrap input element].

AlexJitianu commented 5 years ago

I've followed @AirQuick's suggestions and I've replaced test:ws with a string just like the HTML report does. The next plugin build will contain the fix.