ClassBuilder depending -in some way- on text

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

ClassBuilder depending -in some way- on text

Guillermo Polito
ClassBuilder>>validateClassName: aString
    "Validate the new class name"
    | allowed |
    aString isSymbol
        ifFalse: [ ^ false ].
    allowed := ($0 to: $9), {$_}, ($A to: $Z), ($a to: $z).
    (aString detect: [:c | (allowed includes: c) not] ifNone: [ ])
        ifNotNil: [ :c | self error: 'Invalid character: ''', c printString, ''''.
            ^ false].
    aString first canBeGlobalVarInitial ifFalse:[
        self error: 'Class names must be capitalized'.
        ^false].
    environ at: aString ifPresent:[:old|
        (old isKindOf: Behavior) ifFalse:[
            self notify: aString asText allBold,
                        ' already exists!\Proceed will store over it.' withCRs]].
    ^ true


Is it right to do that asText allBold there? :S
Shouldn't an exception be raised instead?

Do I open a ticket?

Guille
Reply | Threaded
Open this post in threaded view
|

Re: ClassBuilder depending -in some way- on text

Igor Stasenko
On 9 March 2012 01:22, Guillermo Polito <[hidden email]> wrote:

> ClassBuilder>>validateClassName: aString
>     "Validate the new class name"
>     | allowed |
>     aString isSymbol
>         ifFalse: [ ^ false ].
>     allowed := ($0 to: $9), {$_}, ($A to: $Z), ($a to: $z).
>     (aString detect: [:c | (allowed includes: c) not] ifNone: [ ])
>         ifNotNil: [ :c | self error: 'Invalid character: ''', c printString,
> ''''.
>             ^ false].
>     aString first canBeGlobalVarInitial ifFalse:[
>         self error: 'Class names must be capitalized'.
>         ^false].
>     environ at: aString ifPresent:[:old|
>         (old isKindOf: Behavior) ifFalse:[
>             self notify: aString asText allBold,
>                         ' already exists!\Proceed will store over it.'
> withCRs]].
>     ^ true
>
>
> Is it right to do that asText allBold there? :S
> Shouldn't an exception be raised instead?
>
> Do I open a ticket?
>
Yes.
We should kill dependency by removing asText asBold.
Notifications should not accept rich-formatted text anyways,
because not all ui managers can handle it  (now we having command-line
ui manager
which prints message on console).

> Guille



--
Best regards,
Igor Stasenko.

cbc
Reply | Threaded
Open this post in threaded view
|

Re: ClassBuilder depending -in some way- on text

cbc
On Thu, Mar 8, 2012 at 5:17 PM, Igor Stasenko <[hidden email]> wrote:
> On 9 March 2012 01:22, Guillermo Polito <[hidden email]> wrote:
>> ClassBuilder>>validateClassName: aString
...

>>             self notify: aString asText allBold,
>>                         ' already exists!\Proceed will store over it.'
>>
>> Is it right to do that asText allBold there? :S
>> Shouldn't an exception be raised instead?
>>
>> Do I open a ticket?
>>
> Yes.
> We should kill dependency by removing asText asBold.
> Notifications should not accept rich-formatted text anyways,
> because not all ui managers can handle it  (now we having command-line
> ui manager
> which prints message on console).
>
Maybe have the command-line ui manager dumb down the representation -
I'm sure this isn't the only place.

Guille,
this likely pre-dates Exceptions (or at least wide-spread use of
them).  A much better direction is to use exceptions (or
Announcements?) here, and have a process capture them and display
appropriately for what they can display. The mechanism should probably
include Class name and exception text, which could be rich-formatted
as above, or plain formatted for command-line UI, or interrogated for
other observers.

Just suggestions.

-Chris

Reply | Threaded
Open this post in threaded view
|

Re: ClassBuilder depending -in some way- on text

Stéphane Ducasse
In reply to this post by Guillermo Polito
the beauty of layer tangling!
I love that you have a look at that.
I realllllly mean it.

Everyday we are getting closer to a nice and modular system (even if I got bashed by people not believe the effort we spent on that).

Stef


> ClassBuilder>>validateClassName: aString
>     "Validate the new class name"
>     | allowed |
>     aString isSymbol
>         ifFalse: [ ^ false ].
>     allowed := ($0 to: $9), {$_}, ($A to: $Z), ($a to: $z).
>     (aString detect: [:c | (allowed includes: c) not] ifNone: [ ])
>         ifNotNil: [ :c | self error: 'Invalid character: ''', c printString, ''''.
>             ^ false].
>     aString first canBeGlobalVarInitial ifFalse:[
>         self error: 'Class names must be capitalized'.
>         ^false].
>     environ at: aString ifPresent:[:old|
>         (old isKindOf: Behavior) ifFalse:[
>             self notify: aString asText allBold,
>                         ' already exists!\Proceed will store over it.' withCRs]].
>     ^ true
>
>
> Is it right to do that asText allBold there? :S
> Shouldn't an exception be raised instead?
>
> Do I open a ticket?
>
> Guille


Reply | Threaded
Open this post in threaded view
|

Re: ClassBuilder depending -in some way- on text

Igor Stasenko
In reply to this post by cbc
On 9 March 2012 04:28, Chris Cunningham <[hidden email]> wrote:

> On Thu, Mar 8, 2012 at 5:17 PM, Igor Stasenko <[hidden email]> wrote:
>> On 9 March 2012 01:22, Guillermo Polito <[hidden email]> wrote:
>>> ClassBuilder>>validateClassName: aString
> ...
>>>             self notify: aString asText allBold,
>>>                         ' already exists!\Proceed will store over it.'
>>>
>>> Is it right to do that asText allBold there? :S
>>> Shouldn't an exception be raised instead?
>>>
>>> Do I open a ticket?
>>>
>> Yes.
>> We should kill dependency by removing asText asBold.
>> Notifications should not accept rich-formatted text anyways,
>> because not all ui managers can handle it  (now we having command-line
>> ui manager
>> which prints message on console).
>>
> Maybe have the command-line ui manager dumb down the representation -
> I'm sure this isn't the only place.
>
> Guille,
> this likely pre-dates Exceptions (or at least wide-spread use of
> them).  A much better direction is to use exceptions (or
> Announcements?) here, and have a process capture them and display
> appropriately for what they can display. The mechanism should probably
> include Class name and exception text, which could be rich-formatted
> as above, or plain formatted for command-line UI, or interrogated for
> other observers.
>
if you replace it with exception, it doesn't means that exception should
use text and bold..
so...

> Just suggestions.
>
> -Chris
>



--
Best regards,
Igor Stasenko.

Reply | Threaded
Open this post in threaded view
|

Re: ClassBuilder depending -in some way- on text

Guillermo Polito
I tagged it as 1.5 ;)

http://code.google.com/p/pharo/issues/detail?id=5456&thanks=5456&ts=1331306643

On Fri, Mar 9, 2012 at 7:09 AM, Igor Stasenko <[hidden email]> wrote:
On 9 March 2012 04:28, Chris Cunningham <[hidden email]> wrote:
> On Thu, Mar 8, 2012 at 5:17 PM, Igor Stasenko <[hidden email]> wrote:
>> On 9 March 2012 01:22, Guillermo Polito <[hidden email]> wrote:
>>> ClassBuilder>>validateClassName: aString
> ...
>>>             self notify: aString asText allBold,
>>>                         ' already exists!\Proceed will store over it.'
>>>
>>> Is it right to do that asText allBold there? :S
>>> Shouldn't an exception be raised instead?
>>>
>>> Do I open a ticket?
>>>
>> Yes.
>> We should kill dependency by removing asText asBold.
>> Notifications should not accept rich-formatted text anyways,
>> because not all ui managers can handle it  (now we having command-line
>> ui manager
>> which prints message on console).
>>
> Maybe have the command-line ui manager dumb down the representation -
> I'm sure this isn't the only place.
>
> Guille,
> this likely pre-dates Exceptions (or at least wide-spread use of
> them).  A much better direction is to use exceptions (or
> Announcements?) here, and have a process capture them and display
> appropriately for what they can display. The mechanism should probably
> include Class name and exception text, which could be rich-formatted
> as above, or plain formatted for command-line UI, or interrogated for
> other observers.
>
if you replace it with exception, it doesn't means that exception should
use text and bold..
so...

> Just suggestions.
>
> -Chris
>



--
Best regards,
Igor Stasenko.


cbc
Reply | Threaded
Open this post in threaded view
|

Re: ClassBuilder depending -in some way- on text

cbc
In reply to this post by Igor Stasenko
On Fri, Mar 9, 2012 at 2:09 AM, Igor Stasenko <[hidden email]> wrote:

> On 9 March 2012 04:28, Chris Cunningham <[hidden email]> wrote:
>>
>> Guille,
>> this likely pre-dates Exceptions (or at least wide-spread use of
>> them).  A much better direction is to use exceptions (or
>> Announcements?) here, and have a process capture them and display
>> appropriately for what they can display. The mechanism should probably
>> include Class name and exception text, which could be rich-formatted
>> as above, or plain formatted for command-line UI, or interrogated for
>> other observers.
>>
> if you replace it with exception, it doesn't means that exception should
> use text and bold..
> so...
>
Right, if with exception, just the raw data needed, no fancy
formatting (that's for the UI or presentation to handle, if it wants
too).