On Sun, 10 Jan 2010, Andreas Raab wrote:

> Levente Uzonyi wrote:
>> On Sun, 10 Jan 2010, Levente Uzonyi wrote:
>> 
>>> On Sun, 10 Jan 2010, Andreas Raab wrote:
>>> 
>>>> Levente Uzonyi wrote:
>>>>> It works, but there's a new (minor) issue: the diff contains double line 
>>>>> endings. And the old green image was better than the current blue on the 
>>>>> web interface, because other style elements are for the green image.
>>>> 
>>>> Both should be fixed now.
>>> 
>>> There are (at least) three ways to fix the extra empty lines issue:
>>> 1. restore the original behavior in TextDiffBuilder >> #split: by using
>>>  endWithoutSeparators instead of end.
>>> 2. update TextDiffBuilder >> #printTextPatchSequence:on: to add cr if and
>>>  only if the line doesn't end with cr or crlf.
>>> 3. create a subclass of TextDiffBuilder that overrides #split: and throws
>>>  away line endings. and use that from MCDiffyTextWriter. The extension
>>>  methods could be moved there too.
>> 
>> I finally fixed it in a fourth way by making #buildPatchSequence more 
>> backwards compatible by ignoring crs (System-ul.230). A fifth solution 
>> would be to change #buildTextPatch to use 
>> #patchSequenceDoIfMatch:ifInsert:ifRemove: directly, like:
>> 
>> TextDiffBuilder >> #buildTextPatch
>>
>>    ^String streamContents: [ :stream |
>>       self
>>          patchSequenceDoIfMatch: [ :string |
>>             stream space: 2.
>>             self print: string withAttributes: nil on: stream ]
>>          ifInsert: [ :string |
>>             stream nextPutAll: '+ '.
>>             self print: string withAttributes: nil on: stream ]
>>          ifRemove: [ :string |
>>             stream nextPutAll: '- '.
>>             self print: string withAttributes: nil on: stream ] ]
>> 
>> This would also allow us to remove #stringForAttributes: and 
>> #printTextPatchSequence:on:.
>
> I think I'll leave that decision to you, you seem to have a good handle on 
> this part of the system. FWIW, I had implemented option #2 for our 
> SqueakSource installation realizing that it would be robust even if we 
> decided to leave out the CRs from the diff.

With the latest TextDiffBuilder changes everything should work fine with 
all versions of SqueakSource. #buildTextPatch is SqueakSource's extension 
method, changing that would break backwards compatibility.


Levente

>
> Cheers,
>  - Andreas
>
>