On 9/1/2010 1:49 PM, Bert Freudenberg wrote:
> On 01.09.2010, at 22:22, Frank Shearar wrote:
>
>> On 2010/09/01 15:38, Bert Freudenberg wrote:
>>> On 01.09.2010, at 10:25, Frank Shearar wrote:
>>>
>>>> On 2010/09/01 07:32, commits at source.squeak.org wrote:
>>>>> Andreas Raab uploaded a new version of Network to project The Trunk:
>>>>> http://source.squeak.org/trunk/Network-ar.82.mcz
>>>>>
>>>>> ==================== Summary ====================
>>>>>
>>>>> Name: Network-ar.82
>>>>> Author: ar
>>>>> Time: 1 September 2010, 12:32:08.542 am
>>>>> UUID: aa52635f-d746-5744-b432-582c5ba62b11
>>>>> Ancestors: Network-ul.81
>>>>>
>>>>> Use HttpUrl when dealing with https, i.e., 'https://encrypted.google.com' asUrl class == HttpUrl
>>>>>
>>>>> =============== Diff against Network-ul.81 ===============
>>>>>
>>>>> Item was changed:
>>>>>    ----- Method: Url class>>urlClassForScheme: (in category 'parsing') -----
>>>>>    urlClassForScheme: scheme
>>>>>    	(scheme isNil or: [scheme = 'http']) ifTrue: [^HttpUrl].
>>>>> + 	scheme = 'https' ifTrue: [^HttpUrl].
>>>>>    	scheme = 'ftp' ifTrue: [^FtpUrl].
>>>>>    	scheme = 'file' ifTrue: [^FileUrl].
>>>>>    	scheme = 'mailto' ifTrue: [^MailtoUrl].
>>>>>    	scheme = 'browser' ifTrue: [^BrowserUrl].
>>>>>    	^GenericUrl!
>>>>
>>>> That reminds me: for my SipStack I would need to touch this method (*), as would any other packages using other kinds of URLs.
>>>>
>>>> Perhaps we need a registry-based approach, so that we can safely load multiple URL-defining packages?
>>>>
>>>> I see no point in having SipUrl become part of Network, because most images won't use it. Protocols like HTTP and FTP are pretty much ubiquitous on the other hand, so it makes sense to keep them in Network(-Url).
>>>>
>>>> frank
>>>
>>> The canonical way would be to iterate through Url subclasses in reverse superclass-order to find the most specialized subclass willing to handle a particular scheme.
>>
>> OK, I hope my attempt's reasonable. Each subclass says what schemes it handles in Foo class>>handlesScheme:. Url class>>urlClassForScheme: finds the most specific subclass that will accept a particular scheme, by finding the first to say "yes please" in Url allSubclasses reverse.
>>
>> The package number's not great though: it should be Network-fbs.83, adding to Network-ar.82.
>>
>> frank
>
> Code looks good. But lets hear from people more familiar with the Network package or URLs if this is the way it should be ...

The code looks okay to me, albeit maybe adding a little more complexity 
than strictly necessary. I think I would prefer a registration mechanism 
(simple map of scheme to url class) over the classes queries because 
there can be situations where you'd like to register schemes with 
existing URL classes and I find it simpler to say (for example)

RTSPClient class>>initialize
	"Make sure that rtsp URLs can be handled"

	Url registerUrlClass: HierarchicalUrl forScheme: 'rtsp'.

SqueakSSL class>>initialize
	"Make sure a suitable URL class is registered for https"

	(Url urlClassForScheme: 'https') == GenericUrl ifTrue:[
		"No good; re-register as HttpUrl"
		Url registerUrlClass: HttpUrl forScheme: 'https'.
	].


instead of having to subclass and implementing #handlesScheme:. But this 
may be just me :-)

Cheers,
   - Andreas