[Cuis-dev] Encoder changes re: #addMultiRange:for: and #noteSourceRange:forNode:

[Phil B]

Hernan,

Thanks for the detail... that clears up how we got here.  So the question
is how to best address the issue?  The problem I'm running into is that I
have an Encoder adding a range to sourceRanges via
#noteSourceRange:forNode: (i.e. a singular range) but later Parser is
attempting to add a range  via #addMultiRange:for: which fails since
Interval DNU #add:.[1]   So we've got a logical inconsistency in that
#noteSourceRange:forNode: can add a singular Interval while
#addMultiRange:for: assumes it will always be a collection of Intervals.
It would be nice if either #noteSourceRange:forNode: raised an
exception/wrapped in a collection if a singular range gets passed to it or
#addMultiRange:for: properly handled not finding a collection of ranges in
sourceRanges.  I don't have any particular preference, but it seems that
the current implementation is at least incomplete.

[1] In my case, it's happening via my subclasses of Encoder and Parser but
appears to be (previously) perfectly valid uses of both.  So either the
default code paths never attempt to do this or we've just gotten lucky so
far? (have you tried recompiling the Image?)

Thanks,
Phil

On Sun, Apr 21, 2019 at 8:10 AM Hernan Wilkinson <
hernan.wilkinson at 10pines.com> wrote:

> Hi Phill,
>  let me tell you the story about it :-)
>  Originally, the encoder only kept the ranges of message nodes because
> they are used in the debugger to highlight the message send when steeping.
>  As part of the implementation of refactorings, I changed the parser to
> keep the ranges of all parse nodes.
>  The thing is, there are parse nodes like a variable node or a literal
> node that can be reused in the AST and therefore for the same node you can
> have multiple ranges, that is where the ordered collection of ranges
> comes from.
>  I did not change how ranges for message send were stored to avoid further
> changes and keep compatibility with the rest of the tools that assumed that.
>  I know it is not nice to get an interval for some node types and a
> collection of intervals for others, they are not polymorphic and therefore
> it breaks encapsulation, but it was the "safest" way to do it due to the
> lack of tests and the difficulty to analyze a major change.
>  In the change set I sent that allows renames in the class definition, I
> added a message to MethodNode, #isMultipleRanges: that encapsulates the
> fact that a node can have one or multiple ranges.
>  Writing this email I realized we could add a new message to get node
> ranges that would always return a collection and at least for that message
> the return would be polymorphic always, no matter the node type... but
> making all the tools to use that message is another story :-)
>
>  Cheers!
>  Hernan.
>
> On Sun, Apr 21, 2019 at 1:55 AM Phil B via Cuis-dev <
> cuis-dev at lists.cuis.st> wrote:
>
>> I see that not too long ago that Encoder was modified to have
>> #addMultiRange:for: use collections of Intervals for ranges in the
>> sourceRanges dictionary.  However, #noteSourceRange:forNode: still adds
>> singular Intervals to sourceRanges which causes problems.  Not sure how to
>> best resolve this... (I tried a quick hack of having it also wrap its
>> Intervals in collections which as expected caused problems elsewhere so
>> figured I'd ask before going further)
>> --
>> Cuis-dev mailing list
>> Cuis-dev at lists.cuis.st
>> https://lists.cuis.st/mailman/listinfo/cuis-dev
>>
>
>
> --
>
> *Hernán WilkinsonAgile Software Development, Teaching & Coaching*
> *Phone: +54-011*-4893-2057
> *Twitter: @HernanWilkinson*
> *site: http://www.10Pines.com <http://www.10pines.com/>*
> Address: Alem 896, Floor 6, Buenos Aires, Argentina
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.cuis.st/mailman/archives/cuis-dev/attachments/20190421/9f3180f5/attachment.html>