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 |
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 |
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
|
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
|
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 |
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
|
Free forum by Nabble | Edit this page |