[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
|