Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 366871

Summary:	[Compare]Converting jsDiff result into mappers.
Product:	[ECD] Orion	Reporter:	libing wang <libingw>
Component:	Client	Assignee:	libing wang <libingw>
Status:	RESOLVED FIXED	QA Contact:
Severity:	normal
Priority:	P3	CC:	simon_kaegi
Version:	0.4
Target Milestone:	0.4 M2
Hardware:	PC
OS:	Windows 7
Whiteboard:
Bug Depends on:	366590
Bug Blocks:

Description libing wang

2011-12-15 15:59:58 EST

To fixed bug 366590, I have been doing unit test against all the 50 cases I wrote before. 
The unit test in bug 366590 was against jsDiff.createPatch function, where a unified diff should be generated and passed to the tester. It turned out that 10 out of 50 failed due to some holes in the API, where it hit some edge cases.
I will send these cases and the failure to the author.
But before it gets fixed I will just use its lower level APIs to convert to the mappers used in the compare widget.
1. The low level API generates all the diffs as an array of object, which is good.
2. It only has removed and added blocks, no changed block.
2. In some cases, e.g., changing 3 lines in a row, it does not generates one removed block followed by an added block(supposed to). Instead, it just generates 3 separated remove blocks followed by 3 add block respectively. In the converter, we need to merge them.

Comment 1 libing wang

2011-12-16 13:44:29 EST

I committed a change on the unit test with 73fb3ea9ea3422d79b8ef36457b4c1446622a256.

It seems that jsDiff treated "\r\n" as two lines while in my unit test it is treated as one line. That is why some of case [23,29,37,39,40,41,42,43,44,49] failed.
But I still have 4 cases[23,29,39,40] failed. 
Those are really rare cases only happens on changing around last line of a 4 line string.
I've sent an email to the author regarding the "\r\n" treatment and the 4 failure cases.

If we can get a quick fix we can still use createPatch.
But converting their diff objects into the mapper object should not be a big deal. So I will just add the converter as another option.

Comment 2 libing wang

2011-12-16 13:47:46 EST

Just want to clarify that at the last bullet in the description, I said 
"changing 3 lines in a row, it does not generates one
removed block followed by an added block(supposed to)"

This is a false statement if I use "\n" instead of "\r\n".
Because if I use "\r\n", jsDiff may treat the "3 lines in a row" as "6 lines".

Comment 3 libing wang

2011-12-19 17:50:00 EST

Added the adapter to adapt the jsdiff objects into Orion comapre mapper.
Tried a trial hook up by using this adapter in Orion, works fine.
http://git.eclipse.org/c/orion/org.eclipse.orion.client.git/commit/?id=59924512fcf21db00c7182fe59f58e12832a27f9


Will add some more unit test on this adapter tomorrow.After that, will close it.

Comment 4 libing wang

2011-12-21 09:59:53 EST

added unite test for the adapter tests.
46 passed on 50 tests.
http://git.eclipse.org/c/orion/org.eclipse.orion.client.git/commit/?id=1435533d1fe415f7b00e9e3c47dc6de6caef7655

Comment 5 libing wang

2011-12-21 10:05:38 EST

Fixed the remaining 4 unit tests.
Now unit test 100% passed on the JSDiff adapter. We are ready to use it for the compare widget.

http://git.eclipse.org/c/orion/org.eclipse.orion.client.git/commit/?id=1d9546a3e5147a8678a14f713d56525a3728007d

We do not have to use the createPatch API but just leave it as a  future option when the bug is fixed to pass test [23,29,39,40].