whisker browser

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

whisker browser

Martin Kuball
Hi!

I haven't used squeak for programing in years (yes, I know it's sad). Now that
I do, I noticed that there is no whisker package for 5.3. Well actually there
seems to be no package at all any more. So my question is, does anybody has a
whisker version that plays well with the current squeak version?

Meanwhile I loaded the version I used in 3.9. Had to fix some deprecated
warnings and some errors during initialization. I have it running now. But
it's missing some of the new features - like the icons - of the system
browser.

While fixing the errors I noticed the following method:

StringMorph>>font: aFont emphasis: emphasisCode

        self
                setFont: ((aFont isNil or: [aFont emphasis = emphasisCode] or:
[emphasisCode isNil])
                        ifTrue: [aFont]
                        ifFalse: [aFont emphasized: emphasisCode])
                emphasis: (emphasisCode ifNil: [aFont emphasis]).

The check (aFont isNil) seems to imply that nil is a valid value here. But if
you try, the method fails at [aFont emphasis].

Martin




Reply | Threaded
Open this post in threaded view
|

Re: whisker browser

K K Subbu
On 24/05/20 6:56 pm, Martin Kuball wrote:

> StringMorph>>font: aFont emphasis: emphasisCode
>
> self
> setFont: ((aFont isNil or: [aFont emphasis = emphasisCode] or:
> [emphasisCode isNil])
> ifTrue: [aFont]
> ifFalse: [aFont emphasized: emphasisCode])
> emphasis: (emphasisCode ifNil: [aFont emphasis]).
>
> The check (aFont isNil) seems to imply that nil is a valid value here. But if
> you try, the method fails at [aFont emphasis].

Good catch! The () for emphasis: is being evaluated without checking for
aFont isNil. The aFont isNil check should be done before setFont.

Please post a patch to Inbox. See http://wiki.squeak.org/squeak/3279 for
the simple procedure.

Regards .. Subbu

Reply | Threaded
Open this post in threaded view
|

Re: whisker browser

marcel.taeumel
In reply to this post by Martin Kuball
Hi Martin.

The check (aFont isNil) seems to imply that nil is a valid value here. But if
you try, the method fails at [aFont emphasis].

Well, either aFont or emphasisCode can be nil. Not both. ;-) If I recall correctly, that's the existing usage I found when refactoring StringMorph last year in summer. It would be strange to support both arguments to be nil.

Background: Actually, the emphasis is part of the font object. However, it is possible to just store the emphasis in StringMorph and then fall back to the default font when needed -- with that emphasis. So, it is a bit of a mess, when you think about it. The user's model might separate font emphasis and font face. But the low-level implementation does not. Only StringMorph does.

Best,
Marcel

Am 24.05.2020 15:26:35 schrieb Martin Kuball <[hidden email]>:

Hi!

I haven't used squeak for programing in years (yes, I know it's sad). Now that
I do, I noticed that there is no whisker package for 5.3. Well actually there
seems to be no package at all any more. So my question is, does anybody has a
whisker version that plays well with the current squeak version?

Meanwhile I loaded the version I used in 3.9. Had to fix some deprecated
warnings and some errors during initialization. I have it running now. But
it's missing some of the new features - like the icons - of the system
browser.

While fixing the errors I noticed the following method:

StringMorph>>font: aFont emphasis: emphasisCode

self
setFont: ((aFont isNil or: [aFont emphasis = emphasisCode] or:
[emphasisCode isNil])
ifTrue: [aFont]
ifFalse: [aFont emphasized: emphasisCode])
emphasis: (emphasisCode ifNil: [aFont emphasis]).

The check (aFont isNil) seems to imply that nil is a valid value here. But if
you try, the method fails at [aFont emphasis].

Martin






Reply | Threaded
Open this post in threaded view
|

Re: whisker browser

marcel.taeumel
In reply to this post by Martin Kuball
Hi Martin,

if you are interesting in alternative program exploration in Squeak/Smalltalk, you might want to also take a look at Vivide. :-)


Best,
Marcel

Am 24.05.2020 15:26:35 schrieb Martin Kuball <[hidden email]>:

Hi!

I haven't used squeak for programing in years (yes, I know it's sad). Now that
I do, I noticed that there is no whisker package for 5.3. Well actually there
seems to be no package at all any more. So my question is, does anybody has a
whisker version that plays well with the current squeak version?

Meanwhile I loaded the version I used in 3.9. Had to fix some deprecated
warnings and some errors during initialization. I have it running now. But
it's missing some of the new features - like the icons - of the system
browser.

While fixing the errors I noticed the following method:

StringMorph>>font: aFont emphasis: emphasisCode

self
setFont: ((aFont isNil or: [aFont emphasis = emphasisCode] or:
[emphasisCode isNil])
ifTrue: [aFont]
ifFalse: [aFont emphasized: emphasisCode])
emphasis: (emphasisCode ifNil: [aFont emphasis]).

The check (aFont isNil) seems to imply that nil is a valid value here. But if
you try, the method fails at [aFont emphasis].

Martin






Reply | Threaded
Open this post in threaded view
|

Re: whisker browser

K K Subbu
In reply to this post by marcel.taeumel
On 25/05/20 12:49 pm, Marcel Taeumel wrote:
> Well, either aFont or emphasisCode can be nil. Not both. ;-) If I recall
> correctly, that's the existing usage I found when refactoring
> StringMorph last year in summer. It would be strange to support both
> arguments to be nil.

StringMorph>>initializeFromText uses 'self font textStyle' without
checking for nil. I saw direct ivar access instead of the fontsToUse
from other places also.

How about this patch?

self assert: (aFont notNil | emphasis notNil).
font := aFont.
emphasis := emphasisCode ifNil: [0].

Regards .. Subbu

Reply | Threaded
Open this post in threaded view
|

Re: whisker browser

marcel.taeumel
Hi Subbu.

I saw direct ivar access instead of the fontsToUse from other places also.

Looks fine. The few ivar reads have ifNil-checks. ivar write is only in #initialize* and #set* methods. Calling #font will never return "nil". So "self font textStyle" is fine.

How about this patch?

Ah, well. You are right. The internals of StringMorph can deal with both font and emphasis being nil. The goal would be to reduce the number of #ifNil checks. If somebody would send #setFont:emphasis: with nil and nil -- which would not be okay because (1) #set* is rather internal initialization code and (2) one of both should be non-nil -- everything would work. However, if somebody would send #font:emphasis: with nil and nil, then the error message is awkward.

You assertion looks good. I will add comments, too.

Best,
Marcel

Am 25.05.2020 10:18:40 schrieb K K Subbu <[hidden email]>:

On 25/05/20 12:49 pm, Marcel Taeumel wrote:
> Well, either aFont or emphasisCode can be nil. Not both. ;-) If I recall
> correctly, that's the existing usage I found when refactoring
> StringMorph last year in summer. It would be strange to support both
> arguments to be nil.

StringMorph>>initializeFromText uses 'self font textStyle' without
checking for nil. I saw direct ivar access instead of the fontsToUse
from other places also.

How about this patch?

self assert: (aFont notNil | emphasis notNil).
font := aFont.
emphasis := emphasisCode ifNil: [0].

Regards .. Subbu