Do you happen to use pretty diffs? If so, you should turn that off. There were quite a few changes which did nothing but reverted := to _. We should revert those again.

Levente

On Wed, 31 Aug 2016, Tim Felgentreff wrote:

You're right, I overlooked this. I just realized how unfortunate the diff format is in this case. None of these methods were changed by me, I merely merged from Squeakland and tried to figure out what to keep :)

On Tue, 30 Aug 2016 at 23:01 Levente Uzonyi leves@caesar.elte.hu wrote: On Tue, 30 Aug 2016, commits@source.squeak.org wrote:

  > Tim Felgentreff uploaded a new version of Network to project The Trunk:
  > http://source.squeak.org/trunk/Network-tfel.184.mcz
  >
  > ==================== Summary ====================
  >
  > Name: Network-tfel.184
  > Author: tfel
  > Time: 30 August 2016, 11:53:57.083946 am
  > UUID: b90acf7c-5796-a347-8939-59955c0588dd
  > Ancestors: Network-tfel.183, Network-ul.183
  >
  > merge a few fixes from Squeakland Etoys.
  > - ServerDirectories always use forward slashes, even on windows
  > - FTPClient connections should go through the NetNameResolver
  > - sockets can only accept if they are connected.
  >
  > =============== Diff against Network-ul.183 ===============
  >
  > Item was changed:
  >  ----- Method: FTPClient>>openPassiveDataConnection (in category 'private protocol') -----
  >  openPassiveDataConnection
  >       | portInfo list dataPort remoteHostAddress |
  >       self sendCommand: 'PASV'.
  >       self lookForCode: 227 ifDifferent: [:response | (TelnetProtocolError protocolInstance: self) signal: 'Could not enter passive
  mode: ' , response].
  > -
  >       portInfo := (self lastResponse findTokens: '()') at: 2.
  >       list := portInfo findTokens: ','.
  > +     remoteHostAddress := NetNameResolver addressForName: (list at: 1)
  > +                                     , '.'
  > +                                     , (list at: 2) , '.'
  > +                                     , (list at: 3) , '.'
  > +                                     , (list at: 4) timeout: 30.
  > -     remoteHostAddress := ByteArray
  > -             with: (list at: 1) asNumber
  > -             with: (list at: 2) asNumber
  > -             with: (list at: 3) asNumber
  > -             with: (list at: 4) asNumber.
  >       dataPort := (list at: 5) asNumber * 256 + (list at: 6) asNumber.
  > +     self openDataSocket: remoteHostAddress port: dataPort!
  > -     self openDataSocket: remoteHostAddress port: dataPort
  > - !
  >
  > Item was changed:
  >  ----- Method: ServerDirectory>>isRoot (in category 'testing') -----
  >  isRoot
  > +     ^ directory = '/'!
  > -     ^directory = (String with: self pathNameDelimiter)!
  >
  > Item was changed:
  >  ----- Method: Socket>>waitForAcceptFor:ifTimedOut: (in category 'waiting') -----
  >  waitForAcceptFor: timeout ifTimedOut: timeoutBlock
  >       "Wait and accept an incoming connection"
  >       self waitForConnectionFor: timeout ifTimedOut: [^timeoutBlock value].
  > +     ^self isConnected

  Does this change actually do anything? Won't the previous line return when
  the socket is not connected?

  Levente

  > +             ifTrue:[self accept]
  > + !
  > -     ^self accept!
  >
  >
  >

That's correct. The only method with underscore assignment, other than those which have it commented out, is DateAndTime >> #<.

Levente

On Wed, 31 Aug 2016, Tobias Pape wrote:

On 31.08.2016, at 22:01, Levente Uzonyi leves@caesar.elte.hu wrote:

...

Do you happen to use pretty diffs? If so, you should turn that off. There were quite a few changes which did nothing but reverted := to _. We should revert those again.

These are not in the current versions. The diffs are bogus. Take for example the recently updated Sound package. Part of the last diff we saw was

==================== Summary ====================

Name: Sound-tfel.58 Author: tfel Time: 21 August 2016, 4:48:44.407391 pm UUID: 50e3fdac-34a3-412c-8cd6-bc672a2424ae Ancestors: Sound-mt.57, Sound-tfel.57

merge trunk

=============== Diff against Sound-mt.57 ===============

Item was changed: ----- Method: AbstractSound class>>fileInSoundLibraryNamed: (in category 'sound library-file in/out') ----- fileInSoundLibraryNamed: fileName "File in the sound library with the given file name, and add its contents to the current sound library."

| s newSounds |

• s _ FileStream readOnlyFileNamed: fileName.
• newSounds _ s fileInObjectAndCode.

• s := FileStream oldFileNamed: fileName.
• newSounds := s fileInObjectAndCode. s close. newSounds associationsDo: [:assoc | self storeFiledInSound: assoc value named: assoc key]. AbstractSound updateScorePlayers. Smalltalk garbageCollect. "Large objects may have been released"

!

Yet, in the _latest_ Package Sound-tfel.59, the method looks like that:

fileInSoundLibraryNamed: fileName "File in the sound library with the given file name, and add its contents to the current sound library."

| s newSounds | s := FileStream readOnlyFileNamed: fileName. newSounds := s fileInObjectAndCode.

(besides, with a comment stamp of: 'kks 9/27/2007 16:25 sound library-file in/out' )

=======================================================================

So, what tim did was to commit the most recent version of the changes he had and retroactively pulling in the ancestors. The only problem here is SqueakSource, which does not shut up about this. Those Diffs are just meaningless.

Best regards -Tobias

...

Levente

On Wed, 31 Aug 2016, Tim Felgentreff wrote:

...

You're right, I overlooked this. I just realized how unfortunate the diff format is in this case. None of these methods were changed by me, I merely merged from Squeakland and tried to figure out what to keep :) On Tue, 30 Aug 2016 at 23:01 Levente Uzonyi leves@caesar.elte.hu wrote: On Tue, 30 Aug 2016, commits@source.squeak.org wrote:

> Tim Felgentreff uploaded a new version of Network to project The Trunk:
> http://source.squeak.org/trunk/Network-tfel.184.mcz
>
> ==================== Summary ====================
>
> Name: Network-tfel.184
> Author: tfel
> Time: 30 August 2016, 11:53:57.083946 am
> UUID: b90acf7c-5796-a347-8939-59955c0588dd
> Ancestors: Network-tfel.183, Network-ul.183
>
> merge a few fixes from Squeakland Etoys.
> - ServerDirectories always use forward slashes, even on windows
> - FTPClient connections should go through the NetNameResolver
> - sockets can only accept if they are connected.
>
> =============== Diff against Network-ul.183 ===============
>
> Item was changed:
>  ----- Method: FTPClient>>openPassiveDataConnection (in category 'private protocol') -----
>  openPassiveDataConnection
>       | portInfo list dataPort remoteHostAddress |
>       self sendCommand: 'PASV'.
>       self lookForCode: 227 ifDifferent: [:response | (TelnetProtocolError protocolInstance: self) signal: 'Could not enter passive
 mode: ' , response].
> -
>       portInfo := (self lastResponse findTokens: '()') at: 2.
>       list := portInfo findTokens: ','.
> +     remoteHostAddress := NetNameResolver addressForName: (list at: 1)
> +                                     , '.'
> +                                     , (list at: 2) , '.'
> +                                     , (list at: 3) , '.'
> +                                     , (list at: 4) timeout: 30.
> -     remoteHostAddress := ByteArray
> -             with: (list at: 1) asNumber
> -             with: (list at: 2) asNumber
> -             with: (list at: 3) asNumber
> -             with: (list at: 4) asNumber.
>       dataPort := (list at: 5) asNumber * 256 + (list at: 6) asNumber.
> +     self openDataSocket: remoteHostAddress port: dataPort!
> -     self openDataSocket: remoteHostAddress port: dataPort
> - !
>
> Item was changed:
>  ----- Method: ServerDirectory>>isRoot (in category 'testing') -----
>  isRoot
> +     ^ directory = '/'!
> -     ^directory = (String with: self pathNameDelimiter)!
>
> Item was changed:
>  ----- Method: Socket>>waitForAcceptFor:ifTimedOut: (in category 'waiting') -----
>  waitForAcceptFor: timeout ifTimedOut: timeoutBlock
>       "Wait and accept an incoming connection"
>       self waitForConnectionFor: timeout ifTimedOut: [^timeoutBlock value].
> +     ^self isConnected

 Does this change actually do anything? Won't the previous line return when
 the socket is not connected?

 Levente

> +             ifTrue:[self accept]
> + !
> -     ^self accept!
>
>
>