[squeak-dev] The Inbox: ShoutCore-nice.63.mcz

Levente Uzonyi leves at caesar.elte.hu
Thu Apr 11 12:57:05 UTC 2019


On Sun, 7 Apr 2019, Nicolas Cellier wrote:

> There are not many ShoutTests that could ensure that I'm no the right track...
> And Shout code is hard to read for me (see below).
> It's also strange to have to change such a central method after all these years of service.
> Many reasons for a stage in inbox, if ever a few pairs of eyes are browsing there...

The change looks correct and works. I suggest it be pushed to the Trunk.

> 
> <rant>
> I find this Shout Parser selector particularly misleading...
> It's hard to guess that #isSelectorCharacter:  is going to check exclusively for binary selectors before I browse the implementation details.
> Should it be named #isBinarySelectorCharacter:, I could eventually omit such browsing.
> 
> In a lesser way, I also do not like self isName, self isBinary, self isKeyword,...
> The intention is to check for the currentToken, not self.
> They have the advantage of being short, but reading the code gives a strange taste.
> If I wanted to avoid the heavy isCurrentTokenAName, isCurrenTokenABinarySelector, isCurrentTokenAKeyword, ... maybe I would just use has instead of is?
> 
> I've also ran the Shout Parser through a Debugger, and I encountered a lot of redundancy during the tokens scan (those isName isKeyword... are ran several consecutive times).

I started rewriting these methods. I'll push the changes to the inbox 
as soon as I'm happy with them.

Levente

> </rant>
> 
> Le dim. 7 avr. 2019 à 15:24, <commits at source.squeak.org> a écrit :
>       Nicolas Cellier uploaded a new version of ShoutCore to project The Inbox:
>       http://source.squeak.org/inbox/ShoutCore-nice.63.mcz
>
>       ==================== Summary ====================
>
>       Name: ShoutCore-nice.63
>       Author: nice
>       Time: 7 April 2019, 3:24:31.721529 pm
>       UUID: 7440b898-b497-493b-adfa-67aef04e0980
>       Ancestors: ShoutCore-eem.62
>
>       Attempt to fix highlighting of 1to:-1
>
>       I don't see the point of testing the presence of a binary selector character after a colon $:, except maybe the case of assignment :=, so only test that case.
>
>       =============== Diff against ShoutCore-eem.62 ===============
>
>       Item was changed:
>         ----- Method: SHParserST80>>scanIdentifier (in category 'scan') -----
>         scanIdentifier
>
>               | c start |
>               start := sourcePosition.
>               [
>                       (c := self nextChar) isAlphaNumeric
>                               or: [ allowUnderscoreSelectors and: [ c == $_ ] ] ] whileTrue.
>       +       (c == $: and: [ self peekChar ~= $= ])
>       -       (c == $: and: [ (self isSelectorCharacter: self peekChar) not ])
>                       ifTrue: [ self nextChar ].
>               currentToken := source copyFrom: start to: sourcePosition - 1.
>               currentTokenSourcePosition := start!
> 
> 
> 
>


More information about the Squeak-dev mailing list