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@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!