<div dir="ltr">Hi Phil!<div> I agree that we should convert the value of the ranges dictionary to a collection of intervals and make them polymorphic once and for all. Sadly that is a change that requires time and great care because it is used in many places (parser, debugger, etc). </div><div> I think the best change would be for AST nodes to know their range and get rid of that dictionary in the encoder, that is something that Nahuel has been analyzing lately but again, it is not an easy change.</div><div> </div><div>I would like to avoid the problems we are generating to you with these changes... are there tests we can run to be sure we have not break you anything?</div><div><br></div><div>Cheers!</div><div>Hernan.</div><div><br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, May 28, 2020 at 3:48 AM Phil B via Cuis-dev <<a href="mailto:cuis-dev@lists.cuis.st">cuis-dev@lists.cuis.st</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div dir="ltr">Nahuel,<div><br></div><div>Thanks for the response... it gave me a chance to calm down and take a closer look at what you changed...</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, May 27, 2020 at 5:51 PM Nahuel Garbezza <<a href="mailto:n.garbezza@gmail.com" target="_blank">n.garbezza@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">Hi Phil,<div><br></div><div>As you were mentioning, the update #4156 enforces that #noteSourceRange:forNode: should always receive an interval instead of a collection. Before this update it worked with both intervals and collection of intervals, kind of a "weird polymorphism" case (because the object was only put into a dictionary, we didn't have any issues). When I looked at all the senders of that message, I saw that we are always passing a single range instead of a collection, that's why I went ahead with the change.</div></div></blockquote><div><br></div><div>My issue with these changes since they were originally made was that the interface and internal representation, while simple, are rather ugly IMO.  It looks like your changes are addressing the ambiguous interface (to a degree) but not the internal representation, which senders must still be aware of.  My code being outside of the changesets gets to see the ugliness first hand...</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div><br></div><div>I'm not sure if I understood correctly the issue you are having. Have you tried using #addMultiRange:for: message? this one is for the nodes that should have collections of source ranges (the nodes that have multiple occurrences on the same AST, like variables). The reason for having two messages for adding source ranges is for the user to explicitly decide when to report a multi range vs. report a single range. Another option could be to have a message like #noteSourceRanges:forNode: (note the plural) to be used with a collection of source ranges, and report all source ranges at once.</div></div></blockquote><div><br></div><div>The issue is the parallel, yet incompatible, accessors of the same ivar (sourceRanges) depending on whether you want a singular range or a multi range.  As a result, the sender of the setter and getter must be coordinated on a per-node basis or you will have problems... which is what was happening with my code.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div><br></div><div>Happy to discuss alternatives to consider all possible use cases!</div></div></blockquote><div><br></div><div>Why not just convert everything to use multi ranges (i.e. internally *always* store a collection in sourceRanges) and eliminate the issue of needing to be aware of the internal representation entirely?</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div><br></div><div>Thanks,</div><div>Nahuel</div></div><br></blockquote><div><br></div><div>Thanks,</div><div>Phil </div></div></div>
-- <br>
Cuis-dev mailing list<br>
<a href="mailto:Cuis-dev@lists.cuis.st" target="_blank">Cuis-dev@lists.cuis.st</a><br>
<a href="https://lists.cuis.st/mailman/listinfo/cuis-dev" rel="noreferrer" target="_blank">https://lists.cuis.st/mailman/listinfo/cuis-dev</a><br>
</blockquote></div><br clear="all"><div><br></div>-- <br><div dir="ltr" class="gmail_signature"><div dir="ltr"><div><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div><span style="font-family:tahoma,sans-serif;font-size:xx-small;border-collapse:collapse"><strong><span style="font-size:8pt"><span><span style="font-size:small"><font size="2"><span style="font-weight:normal"><span style="font-weight:bold">Hernán Wilkinson</span><br>Agile Software Development, Teaching & Coaching</span></font></span></span></span></strong></span></div><div><span style="font-family:tahoma,sans-serif;font-size:xx-small;border-collapse:collapse"><strong><span style="font-size:8pt"><span><span style="font-size:small"><font size="2"><span style="font-weight:normal">Phone: +54-011</span></font></span></span></span></strong></span><font face="tahoma, sans-serif" size="2">-4893-2057</font></div><div><strong style="font-family:tahoma,sans-serif;font-size:xx-small"><span style="font-size:8pt"><span style="font-size:small"><font size="2"><span style="font-weight:normal">Twitter: @HernanWilkinson</span></font></span></span></strong></div><div><span style="font-family:tahoma,sans-serif;font-size:xx-small;border-collapse:collapse"><strong><span style="font-size:8pt"><span><span style="font-size:small"><font size="2"><span style="font-weight:normal">site: <a href="http://www.10pines.com/" style="color:rgb(17,65,112)" target="_blank">http://www.10Pines.com</a></span></font></span></span></span></strong></span></div><div><font face="tahoma, sans-serif"><span style="border-collapse:collapse">Address: Alem 896</span></font>, Floor 6, Buenos Aires, Argentina</div></div></div></div></div></div></div></div></div></div></div></div>