The Inbox: ShoutCore-nice.63.mcz

>
> <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).

> </rant>
>
> Le dim. 7 avr. 2019 à 15:24, <[hidden email]> 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!
>
>
>
>

> On Sun, 7 Apr 2019, Nicolas Cellier wrote:
>
>>
>> <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.