The Trunk: Traits-ar.278.mcz

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
34 messages Options
12
Reply | Threaded
Open this post in threaded view
|

Re: Re: New trunk server

Igor Stasenko
2010/1/12 Levente Uzonyi <[hidden email]>:
> On Mon, 11 Jan 2010, Nicolas Cellier wrote:
>
>> Hi Levente,
>> what about completely ignoring line endings in diffs ?
>>
>
> I intentionally added this feature. Do you think it's wrong?
>
>
IMO empty lines (and white space in general), is not an informal part
of source code,
so diffing them makes not much sense.

> Levente
>
>> split: aString
>>        "I return an Array of strings which are the lines extracted from
>> aString. All lines contain the line separator characters"
>>
>>        ^Array streamContents: [ :stream |
>>                aString lineIndicesDo: [ :start :endWithoutSeparators :end
>> |
>>                        stream nextPut: (aString copyFrom: start to:
>> endWithoutSeparators) ] ]
>>
>> or simply
>>
>>
>> split: aString
>>        "I return an Array of strings which are the lines extracted from
>> aString. All lines contain the line separator characters"
>>
>>        ^Array streamContents: [ :stream |
>>                aString linesDo: [ :aLineWithoutEnding |
>>                        stream nextPut: aLineWithoutEnding ] ]
>>
>> Nicolas
>>
>> 2010/1/11 Andreas Raab <[hidden email]>:
>>>
>>> Levente Uzonyi wrote:
>>>>
>>>> On Sun, 10 Jan 2010, Andreas Raab wrote:
>>>>>
>>>>> 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.
>>>
>>> Yup, agreed. It was a quick localized fix for our installation. I didn't
>>> feel like messing around with TextDiffBuilder itself - making the change
>>> in
>>> the one extension method felt safer for our purposes. Thanks for fixing
>>> the
>>> issue at the root!
>>>
>>> Cheers,
>>>  - Andreas
>>>
>>>
>>
>
>
>
>



--
Best regards,
Igor Stasenko AKA sig.

Reply | Threaded
Open this post in threaded view
|

Re: Re: New trunk server

Levente Uzonyi-2
On Tue, 12 Jan 2010, Igor Stasenko wrote:

> 2010/1/12 Levente Uzonyi <[hidden email]>:
>> On Mon, 11 Jan 2010, Nicolas Cellier wrote:
>>
>>> Hi Levente,
>>> what about completely ignoring line endings in diffs ?
>>>
>>
>> I intentionally added this feature. Do you think it's wrong?
>>
>>
> IMO empty lines (and white space in general), is not an informal part
> of source code,
> so diffing them makes not much sense.
Imagine that you removed lf characters from the code or you
accidentally added some linefeeds while pasting code from and external
source. The diff shows no changes. Is that OK?


Levente

>
>> Levente
>>
>>> split: aString
>>>        "I return an Array of strings which are the lines extracted from
>>> aString. All lines contain the line separator characters"
>>>
>>>        ^Array streamContents: [ :stream |
>>>                aString lineIndicesDo: [ :start :endWithoutSeparators :end
>>> |
>>>                        stream nextPut: (aString copyFrom: start to:
>>> endWithoutSeparators) ] ]
>>>
>>> or simply
>>>
>>>
>>> split: aString
>>>        "I return an Array of strings which are the lines extracted from
>>> aString. All lines contain the line separator characters"
>>>
>>>        ^Array streamContents: [ :stream |
>>>                aString linesDo: [ :aLineWithoutEnding |
>>>                        stream nextPut: aLineWithoutEnding ] ]
>>>
>>> Nicolas
>>>
>>> 2010/1/11 Andreas Raab <[hidden email]>:
>>>>
>>>> Levente Uzonyi wrote:
>>>>>
>>>>> On Sun, 10 Jan 2010, Andreas Raab wrote:
>>>>>>
>>>>>> 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.
>>>>
>>>> Yup, agreed. It was a quick localized fix for our installation. I didn't
>>>> feel like messing around with TextDiffBuilder itself - making the change
>>>> in
>>>> the one extension method felt safer for our purposes. Thanks for fixing
>>>> the
>>>> issue at the root!
>>>>
>>>> Cheers,
>>>>  - Andreas
>>>>
>>>>
>>>
>>
>>
>>
>>
>
>
>
> --
> Best regards,
> Igor Stasenko AKA sig.
>
>

Reply | Threaded
Open this post in threaded view
|

Re: Re: New trunk server

Nicolas Cellier
2010/1/12 Levente Uzonyi <[hidden email]>:

> On Tue, 12 Jan 2010, Igor Stasenko wrote:
>
>> 2010/1/12 Levente Uzonyi <[hidden email]>:
>>>
>>> On Mon, 11 Jan 2010, Nicolas Cellier wrote:
>>>
>>>> Hi Levente,
>>>> what about completely ignoring line endings in diffs ?
>>>>
>>>
>>> I intentionally added this feature. Do you think it's wrong?
>>>
>>>
>> IMO empty lines (and white space in general), is not an informal part
>> of source code,
>> so diffing them makes not much sense.
>
> Imagine that you removed lf characters from the code or you accidentally
> added some linefeeds while pasting code from and external source. The diff
> shows no changes. Is that OK?
>

To me, it's OK since different lineEnding are now rendered the same.
Unless the difference is important for exporting code?

Nicolas

>
> Levente
>
>>
>>> Levente
>>>
>>>> split: aString
>>>>        "I return an Array of strings which are the lines extracted from
>>>> aString. All lines contain the line separator characters"
>>>>
>>>>        ^Array streamContents: [ :stream |
>>>>                aString lineIndicesDo: [ :start :endWithoutSeparators
>>>> :end
>>>> |
>>>>                        stream nextPut: (aString copyFrom: start to:
>>>> endWithoutSeparators) ] ]
>>>>
>>>> or simply
>>>>
>>>>
>>>> split: aString
>>>>        "I return an Array of strings which are the lines extracted from
>>>> aString. All lines contain the line separator characters"
>>>>
>>>>        ^Array streamContents: [ :stream |
>>>>                aString linesDo: [ :aLineWithoutEnding |
>>>>                        stream nextPut: aLineWithoutEnding ] ]
>>>>
>>>> Nicolas
>>>>
>>>> 2010/1/11 Andreas Raab <[hidden email]>:
>>>>>
>>>>> Levente Uzonyi wrote:
>>>>>>
>>>>>> On Sun, 10 Jan 2010, Andreas Raab wrote:
>>>>>>>
>>>>>>> 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.
>>>>>
>>>>> Yup, agreed. It was a quick localized fix for our installation. I
>>>>> didn't
>>>>> feel like messing around with TextDiffBuilder itself - making the
>>>>> change
>>>>> in
>>>>> the one extension method felt safer for our purposes. Thanks for fixing
>>>>> the
>>>>> issue at the root!
>>>>>
>>>>> Cheers,
>>>>>  - Andreas
>>>>>
>>>>>
>>>>
>>>
>>>
>>>
>>>
>>
>>
>>
>> --
>> Best regards,
>> Igor Stasenko AKA sig.
>>
>
>
>
>

Reply | Threaded
Open this post in threaded view
|

Re: Re: New trunk server

Igor Stasenko
In reply to this post by Levente Uzonyi-2
2010/1/12 Levente Uzonyi <[hidden email]>:

> On Tue, 12 Jan 2010, Igor Stasenko wrote:
>
>> 2010/1/12 Levente Uzonyi <[hidden email]>:
>>>
>>> On Mon, 11 Jan 2010, Nicolas Cellier wrote:
>>>
>>>> Hi Levente,
>>>> what about completely ignoring line endings in diffs ?
>>>>
>>>
>>> I intentionally added this feature. Do you think it's wrong?
>>>
>>>
>> IMO empty lines (and white space in general), is not an informal part
>> of source code,
>> so diffing them makes not much sense.
>
> Imagine that you removed lf characters from the code or you accidentally
> added some linefeeds while pasting code from and external source. The diff
> shows no changes. Is that OK?
>

If the new lines is informal part of source code, i.e. belong to the
string literal:

foo := '1
2
3

4
'.

Then we should care. Otherwise not.


>
> Levente
>
>>
>>> Levente
>>>
>>>> split: aString
>>>>        "I return an Array of strings which are the lines extracted from
>>>> aString. All lines contain the line separator characters"
>>>>
>>>>        ^Array streamContents: [ :stream |
>>>>                aString lineIndicesDo: [ :start :endWithoutSeparators
>>>> :end
>>>> |
>>>>                        stream nextPut: (aString copyFrom: start to:
>>>> endWithoutSeparators) ] ]
>>>>
>>>> or simply
>>>>
>>>>
>>>> split: aString
>>>>        "I return an Array of strings which are the lines extracted from
>>>> aString. All lines contain the line separator characters"
>>>>
>>>>        ^Array streamContents: [ :stream |
>>>>                aString linesDo: [ :aLineWithoutEnding |
>>>>                        stream nextPut: aLineWithoutEnding ] ]
>>>>
>>>> Nicolas
>>>>
>>>> 2010/1/11 Andreas Raab <[hidden email]>:
>>>>>
>>>>> Levente Uzonyi wrote:
>>>>>>
>>>>>> On Sun, 10 Jan 2010, Andreas Raab wrote:
>>>>>>>
>>>>>>> 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.
>>>>>
>>>>> Yup, agreed. It was a quick localized fix for our installation. I
>>>>> didn't
>>>>> feel like messing around with TextDiffBuilder itself - making the
>>>>> change
>>>>> in
>>>>> the one extension method felt safer for our purposes. Thanks for fixing
>>>>> the
>>>>> issue at the root!
>>>>>
>>>>> Cheers,
>>>>>  - Andreas
>>>>>
>>>>>
>>>>
>>>
>>>
>>>
>>>
>>
>>
>>
>> --
>> Best regards,
>> Igor Stasenko AKA sig.
>>
>
>
>
>



--
Best regards,
Igor Stasenko AKA sig.

Reply | Threaded
Open this post in threaded view
|

Re: Re: New trunk server

Levente Uzonyi-2
On Tue, 12 Jan 2010, Igor Stasenko wrote:

> 2010/1/12 Levente Uzonyi <[hidden email]>:
>> On Tue, 12 Jan 2010, Igor Stasenko wrote:
>>
>>> 2010/1/12 Levente Uzonyi <[hidden email]>:
>>>>
>>>> On Mon, 11 Jan 2010, Nicolas Cellier wrote:
>>>>
>>>>> Hi Levente,
>>>>> what about completely ignoring line endings in diffs ?
>>>>>
>>>>
>>>> I intentionally added this feature. Do you think it's wrong?
>>>>
>>>>
>>> IMO empty lines (and white space in general), is not an informal part
>>> of source code,
>>> so diffing them makes not much sense.
>>
>> Imagine that you removed lf characters from the code or you accidentally
>> added some linefeeds while pasting code from and external source. The diff
>> shows no changes. Is that OK?
>>
>
> If the new lines is informal part of source code, i.e. belong to the
> string literal:
>
> foo := '1
> 2
> 3
>
> 4
> '.
>
> Then we should care. Otherwise not.
I care and we should. Newlines make a difference. I don't want to decode
one liners like:

at: key put: anObject | index assoc | index := self scanFor: key. assoc := array at: index. assoc ifNil: [self atNewIndex: index put: (Association key: key value: anObject)] ifNotNil: [assoc value: anObject]. ^anObject

It's much easier to read this one:

at: key put: anObject

    | index assoc |
    index := self scanFor: key.
    assoc := array at: index.
    assoc
       ifNil: [ self atNewIndex: index put: (Association key: key value: anObject) ]
       ifNotNil: [ assoc value: anObject ].
    ^anObject

For me, every whitespace counts in the above method, but the original
question was not this, but:
Should the diff algorithm care about the line endings or not?
Are these three lines identical or not (in a diff):
foo bar<cr><lf>
foo bar<cr>
foo bar<lf>


Levente

>
>
>>
>> Levente
>>
>>>
>>>> Levente
>>>>
>>>>> split: aString
>>>>>        "I return an Array of strings which are the lines extracted from
>>>>> aString. All lines contain the line separator characters"
>>>>>
>>>>>        ^Array streamContents: [ :stream |
>>>>>                aString lineIndicesDo: [ :start :endWithoutSeparators
>>>>> :end
>>>>> |
>>>>>                        stream nextPut: (aString copyFrom: start to:
>>>>> endWithoutSeparators) ] ]
>>>>>
>>>>> or simply
>>>>>
>>>>>
>>>>> split: aString
>>>>>        "I return an Array of strings which are the lines extracted from
>>>>> aString. All lines contain the line separator characters"
>>>>>
>>>>>        ^Array streamContents: [ :stream |
>>>>>                aString linesDo: [ :aLineWithoutEnding |
>>>>>                        stream nextPut: aLineWithoutEnding ] ]
>>>>>
>>>>> Nicolas
>>>>>
>>>>> 2010/1/11 Andreas Raab <[hidden email]>:
>>>>>>
>>>>>> Levente Uzonyi wrote:
>>>>>>>
>>>>>>> On Sun, 10 Jan 2010, Andreas Raab wrote:
>>>>>>>>
>>>>>>>> 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.
>>>>>>
>>>>>> Yup, agreed. It was a quick localized fix for our installation. I
>>>>>> didn't
>>>>>> feel like messing around with TextDiffBuilder itself - making the
>>>>>> change
>>>>>> in
>>>>>> the one extension method felt safer for our purposes. Thanks for fixing
>>>>>> the
>>>>>> issue at the root!
>>>>>>
>>>>>> Cheers,
>>>>>>  - Andreas
>>>>>>
>>>>>>
>>>>>
>>>>
>>>>
>>>>
>>>>
>>>
>>>
>>>
>>> --
>>> Best regards,
>>> Igor Stasenko AKA sig.
>>>
>>
>>
>>
>>
>
>
>
> --
> Best regards,
> Igor Stasenko AKA sig.
>
>

Reply | Threaded
Open this post in threaded view
|

Re: Re: New trunk server

Igor Stasenko
2010/1/12 Levente Uzonyi <[hidden email]>:

> On Tue, 12 Jan 2010, Igor Stasenko wrote:
>
>> 2010/1/12 Levente Uzonyi <[hidden email]>:
>>>
>>> On Tue, 12 Jan 2010, Igor Stasenko wrote:
>>>
>>>> 2010/1/12 Levente Uzonyi <[hidden email]>:
>>>>>
>>>>> On Mon, 11 Jan 2010, Nicolas Cellier wrote:
>>>>>
>>>>>> Hi Levente,
>>>>>> what about completely ignoring line endings in diffs ?
>>>>>>
>>>>>
>>>>> I intentionally added this feature. Do you think it's wrong?
>>>>>
>>>>>
>>>> IMO empty lines (and white space in general), is not an informal part
>>>> of source code,
>>>> so diffing them makes not much sense.
>>>
>>> Imagine that you removed lf characters from the code or you accidentally
>>> added some linefeeds while pasting code from and external source. The
>>> diff
>>> shows no changes. Is that OK?
>>>
>>
>> If the new lines is informal part of source code, i.e. belong to the
>> string literal:
>>
>> foo := '1
>> 2
>> 3
>>
>> 4
>> '.
>>
>> Then we should care. Otherwise not.
>
> I care and we should. Newlines make a difference. I don't want to decode one
> liners like:
>
> at: key put: anObject | index assoc | index := self scanFor: key. assoc :=
> array at: index. assoc ifNil: [self atNewIndex: index put: (Association key:
> key value: anObject)]   ifNotNil: [assoc value: anObject]. ^anObject
>
> It's much easier to read this one:
>
> at: key put: anObject
>
>   | index assoc |
>   index := self scanFor: key.
>   assoc := array at: index.
>   assoc
>      ifNil: [ self atNewIndex: index put: (Association key: key value:
> anObject) ]
>      ifNotNil: [ assoc value: anObject ].
>   ^anObject
>
> For me, every whitespace counts in the above method, but the original
> question was not this, but:
> Should the diff algorithm care about the line endings or not?
> Are these three lines identical or not (in a diff):
> foo bar<cr><lf>
> foo bar<cr>
> foo bar<lf>
>

No. Please, not again. :) All of them should be treated equally. We're
talking here about source code,
not about binary files.

>
> Levente
>


--
Best regards,
Igor Stasenko AKA sig.

Reply | Threaded
Open this post in threaded view
|

Re: Re: New trunk server

Nicolas Cellier
In reply to this post by Igor Stasenko
2010/1/12 Igor Stasenko <[hidden email]>:

> 2010/1/12 Levente Uzonyi <[hidden email]>:
>> On Tue, 12 Jan 2010, Igor Stasenko wrote:
>>
>>> 2010/1/12 Levente Uzonyi <[hidden email]>:
>>>>
>>>> On Mon, 11 Jan 2010, Nicolas Cellier wrote:
>>>>
>>>>> Hi Levente,
>>>>> what about completely ignoring line endings in diffs ?
>>>>>
>>>>
>>>> I intentionally added this feature. Do you think it's wrong?
>>>>
>>>>
>>> IMO empty lines (and white space in general), is not an informal part
>>> of source code,
>>> so diffing them makes not much sense.
>>
>> Imagine that you removed lf characters from the code or you accidentally
>> added some linefeeds while pasting code from and external source. The diff
>> shows no changes. Is that OK?
>>
>
> If the new lines is informal part of source code, i.e. belong to the
> string literal:
>
> foo := '1
> 2
> 3
>
> 4
> '.
>
> Then we should care. Otherwise not.

True, there is a "semantic" difference as long as cr lf cr-lf have
different semantic. But do they ?
Since invisible characters are not an explicit specification robust to
future editions, my expectations would be low on such code!
My interpretation would be this one: if developer did rely on specific
line-endings then she should care to explicitely specify line-endings.
If she uses invisible specifications, then it means she just want
whatever line-endings.

Thus, I would not event bother with line endings inside literals and
would tend to say: please use appropriate message (like
withSqueakLineEnding)

Nicolas

>
>
>>
>> Levente
>>
>>>
>>>> Levente
>>>>
>>>>> split: aString
>>>>>        "I return an Array of strings which are the lines extracted from
>>>>> aString. All lines contain the line separator characters"
>>>>>
>>>>>        ^Array streamContents: [ :stream |
>>>>>                aString lineIndicesDo: [ :start :endWithoutSeparators
>>>>> :end
>>>>> |
>>>>>                        stream nextPut: (aString copyFrom: start to:
>>>>> endWithoutSeparators) ] ]
>>>>>
>>>>> or simply
>>>>>
>>>>>
>>>>> split: aString
>>>>>        "I return an Array of strings which are the lines extracted from
>>>>> aString. All lines contain the line separator characters"
>>>>>
>>>>>        ^Array streamContents: [ :stream |
>>>>>                aString linesDo: [ :aLineWithoutEnding |
>>>>>                        stream nextPut: aLineWithoutEnding ] ]
>>>>>
>>>>> Nicolas
>>>>>
>>>>> 2010/1/11 Andreas Raab <[hidden email]>:
>>>>>>
>>>>>> Levente Uzonyi wrote:
>>>>>>>
>>>>>>> On Sun, 10 Jan 2010, Andreas Raab wrote:
>>>>>>>>
>>>>>>>> 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.
>>>>>>
>>>>>> Yup, agreed. It was a quick localized fix for our installation. I
>>>>>> didn't
>>>>>> feel like messing around with TextDiffBuilder itself - making the
>>>>>> change
>>>>>> in
>>>>>> the one extension method felt safer for our purposes. Thanks for fixing
>>>>>> the
>>>>>> issue at the root!
>>>>>>
>>>>>> Cheers,
>>>>>>  - Andreas
>>>>>>
>>>>>>
>>>>>
>>>>
>>>>
>>>>
>>>>
>>>
>>>
>>>
>>> --
>>> Best regards,
>>> Igor Stasenko AKA sig.
>>>
>>
>>
>>
>>
>
>
>
> --
> Best regards,
> Igor Stasenko AKA sig.
>
>

Reply | Threaded
Open this post in threaded view
|

Re: Re: New trunk server

Igor Stasenko
2010/1/12 Nicolas Cellier <[hidden email]>:

> 2010/1/12 Igor Stasenko <[hidden email]>:
>> 2010/1/12 Levente Uzonyi <[hidden email]>:
>>> On Tue, 12 Jan 2010, Igor Stasenko wrote:
>>>
>>>> 2010/1/12 Levente Uzonyi <[hidden email]>:
>>>>>
>>>>> On Mon, 11 Jan 2010, Nicolas Cellier wrote:
>>>>>
>>>>>> Hi Levente,
>>>>>> what about completely ignoring line endings in diffs ?
>>>>>>
>>>>>
>>>>> I intentionally added this feature. Do you think it's wrong?
>>>>>
>>>>>
>>>> IMO empty lines (and white space in general), is not an informal part
>>>> of source code,
>>>> so diffing them makes not much sense.
>>>
>>> Imagine that you removed lf characters from the code or you accidentally
>>> added some linefeeds while pasting code from and external source. The diff
>>> shows no changes. Is that OK?
>>>
>>
>> If the new lines is informal part of source code, i.e. belong to the
>> string literal:
>>
>> foo := '1
>> 2
>> 3
>>
>> 4
>> '.
>>
>> Then we should care. Otherwise not.
>
> True, there is a "semantic" difference as long as cr lf cr-lf have
> different semantic. But do they ?
> Since invisible characters are not an explicit specification robust to
> future editions, my expectations would be low on such code!
> My interpretation would be this one: if developer did rely on specific
> line-endings then she should care to explicitely specify line-endings.
> If she uses invisible specifications, then it means she just want
> whatever line-endings.
>
> Thus, I would not event bother with line endings inside literals and
> would tend to say: please use appropriate message (like
> withSqueakLineEnding)
>

+1 . A well-grounded argumentation.

> Nicolas
>




--
Best regards,
Igor Stasenko AKA sig.

Reply | Threaded
Open this post in threaded view
|

Line endings in diffs (was: Re: Re: New trunk server)

Levente Uzonyi-2
In reply to this post by Igor Stasenko
On Tue, 12 Jan 2010, Igor Stasenko wrote:

> 2010/1/12 Levente Uzonyi <[hidden email]>:
>> Are these three lines identical or not (in a diff):
>> foo bar<cr><lf>
>> foo bar<cr>
>> foo bar<lf>
>>
>
> No. Please, not again. :) All of them should be treated equally. We're
> talking here about source code,
> not about binary files.

Again? I doubt this question was raised before. We are talking about
diffs (at least I am).

(About the source code: I don't care how the source code is handled if
formatting is preserved. And no, I don't like pretty-printed versions.)


Levente

>
>>
>> Levente
>>
>
>
> --
> Best regards,
> Igor Stasenko AKA sig.
>
>

Reply | Threaded
Open this post in threaded view
|

Re: Line endings in diffs (was: Re: Re: New trunk server)

Igor Stasenko
2010/1/12 Levente Uzonyi <[hidden email]>:

> On Tue, 12 Jan 2010, Igor Stasenko wrote:
>
>> 2010/1/12 Levente Uzonyi <[hidden email]>:
>>>
>>> Are these three lines identical or not (in a diff):
>>> foo bar<cr><lf>
>>> foo bar<cr>
>>> foo bar<lf>
>>>
>>
>> No. Please, not again. :) All of them should be treated equally. We're
>> talking here about source code,
>> not about binary files.
>
> Again? I doubt this question was raised before. We are talking about diffs
> (at least I am).

There was a lengthly discussion about how to deal with different line
ending sequences in editor.
You luckily missed it :)

>
> (About the source code: I don't care how the source code is handled if
> formatting is preserved. And no, I don't like pretty-printed versions.)
>
>
> Levente
>
>>
>>>
>>> Levente
>>>
>>
>>
>> --
>> Best regards,
>> Igor Stasenko AKA sig.
>>
>>
>
>



--
Best regards,
Igor Stasenko AKA sig.

Reply | Threaded
Open this post in threaded view
|

Re: Line endings in diffs (was: Re: Re: New trunk server)

Levente Uzonyi-2
On Tue, 12 Jan 2010, Igor Stasenko wrote:

> 2010/1/12 Levente Uzonyi <[hidden email]>:
>> On Tue, 12 Jan 2010, Igor Stasenko wrote:
>>
>>> 2010/1/12 Levente Uzonyi <[hidden email]>:
>>>>
>>>> Are these three lines identical or not (in a diff):
>>>> foo bar<cr><lf>
>>>> foo bar<cr>
>>>> foo bar<lf>
>>>>
>>>
>>> No. Please, not again. :) All of them should be treated equally. We're
>>> talking here about source code,
>>> not about binary files.
>>
>> Again? I doubt this question was raised before. We are talking about diffs
>> (at least I am).
>
> There was a lengthly discussion about how to deal with different line
> ending sequences in editor.
> You luckily missed it :)

The context is different. This is about diffs, not editors.


Levente

>
>>
>> (About the source code: I don't care how the source code is handled if
>> formatting is preserved. And no, I don't like pretty-printed versions.)
>>
>>
>> Levente
>>
>>>
>>>>
>>>> Levente
>>>>
>>>
>>>
>>> --
>>> Best regards,
>>> Igor Stasenko AKA sig.
>>>
>>>
>>
>>
>
>
>
> --
> Best regards,
> Igor Stasenko AKA sig.
>
>

Reply | Threaded
Open this post in threaded view
|

Re: Stress test (was: Re: Re: New trunk server)

Mariano Martinez Peck
In reply to this post by Levente Uzonyi-2


On Mon, Jan 11, 2010 at 10:43 AM, Levente Uzonyi <[hidden email]> wrote:
On Sun, 10 Jan 2010, Andreas Raab wrote:

Bert Freudenberg wrote:
Rock!

Let's watch if that fixes the next Multilingual package upload ...

Works all right so far, but I think we still got two problems:

1) Does anyone have an idea where the double line ends come from when mailing out the diffs? It's really annoying. Any pointers to where to start looking at would be greatly welcome.

2) Does anyone feel as if we're getting a higher number of connection failures than usual? It could be an issue on my end but I somehow doubt that. I seem to getting connection failures about 20% of the time when trying to update. It's quite noticable.


I did a bit of stress testing, by uploading a few packages one after another. There was no problem during the uploads, but when all uploads were finished it took 20-30 seconds for the server to respond for the next few 1-2 minutes. I guess it's related to the gc settings, so tweaking them may help. For a reference, here are the "seaside defaults":

SmalltalkImage current
  vmParameterAt: 5 put: 100000;
  vmParameterAt: 6 put: 35000;
  vmParameterAt: 24 put: 16 * 1024 * 1024;
  vmParameterAt: 25 put: 8 * 1024 * 1024.


Levente: were did you get this values??   What is "seaside default" ?

I see another values in my "seaside default" image.

Thanks

Mariano
 
Smalltalk
  setGCBiasToGrowGCLimit: 16 * 1024 * 1024;
  setGCBiasToGrow: 1

The uploaded changes were produced with the code critics feature of OmniBrowser. I think we should use it regularly, because it can point out issues which could be hard to find otherwise and there are plenty of issues to be fixed. They are bit broken at the moment, so running them on the whole image needs a few fixes.
Some issues are easy to fix, others require deep understanding of the system.

Some categories which should be fixed (number of possible issues):
Messages sent but not implemented (571)
Sends unknown message to global (36)
Subclass responsibility not defined (82)
Instance variable capitalization (4)
Temporary variable capitalization (5)
Inconsistent method classification (324)
Non-blocks in special messages (24)
Unnecessary assignment or return in block (45)
Uses "(a and: [b]) and: [c]" instead of "a and: [b and: [c]]" (78)
Uses (to:)do: instead of to:do: (23)
Variable is only assigned a single literal value (44)
Instance variable overridden by temporary variable (21)
Possible missing "; yourself" (490)
Returns a boolean and non boolean (42)
Subclass of collection that has instance variable but doesn't define copyEmpty (1)
Menus missing translations (75)
Method source contains linefeeds (123)
Assignment has no effect (22)
Check for same statements at end of ifTrue:ifFalse: blocks (43)
Instance variables not read AND written (209)
Method just sends super message (17)
Variable referenced in only one method and always assigned first (69)
Variables not referenced (266)

I hope we can decrease these numbers soon.


Levente

Cheers,
 - Andreas






Reply | Threaded
Open this post in threaded view
|

Re: Stress test (was: Re: Re: New trunk server)

Levente Uzonyi-2
On Wed, 13 Jan 2010, Mariano Martinez Peck wrote:

> On Mon, Jan 11, 2010 at 10:43 AM, Levente Uzonyi <[hidden email]> wrote:
>
>> SmalltalkImage current
>>   vmParameterAt: 5 put: 100000;
>>   vmParameterAt: 6 put: 35000;
>>   vmParameterAt: 24 put: 16 * 1024 * 1024;
>>   vmParameterAt: 25 put: 8 * 1024 * 1024.
>>
>
>
> Levente: were did you get this values??   What is "seaside default" ?
>
> I see another values in my "seaside default" image.
>

>From SmalltalkImage >> #initializeMemorySettingsProfileSeaSide.
What are your defaults?


Levente

> Thanks
>
> Mariano
>
>

Reply | Threaded
Open this post in threaded view
|

Re: Stress test (was: Re: Re: New trunk server)

Mariano Martinez Peck


On Wed, Jan 13, 2010 at 4:07 PM, Levente Uzonyi <[hidden email]> wrote:
On Wed, 13 Jan 2010, Mariano Martinez Peck wrote:

On Mon, Jan 11, 2010 at 10:43 AM, Levente Uzonyi <[hidden email]> wrote:

SmalltalkImage current
 vmParameterAt: 5 put: 100000;
 vmParameterAt: 6 put: 35000;
 vmParameterAt: 24 put: 16 * 1024 * 1024;
 vmParameterAt: 25 put: 8 * 1024 * 1024.



Levente: were did you get this values??   What is "seaside default" ?

I see another values in my "seaside default" image.


>From SmalltalkImage >> #initializeMemorySettingsProfileSeaSide.
What are your defaults?



I don't see that method neither in 2.8 or 3  ... wierd
 

Levente

Thanks

Mariano






12