<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=us-ascii">
<meta name="Generator" content="Microsoft Exchange Server">
<!-- converted from text --><style><!-- .EmailQuote { margin-left: 1pt; padding-left: 4pt; border-left: #800000 2px solid; } --></style>
</head>
<body>
<meta content="text/html; charset=UTF-8">
<style type="text/css" style="">
<!--
p
{margin-top:0;
margin-bottom:0}
-->
</style>
<div dir="ltr">
<div id="x_divtagdefaultwrapper" dir="ltr" style="font-size:12pt; color:#000000; font-family:Calibri,Helvetica,sans-serif">
<p>Hello, thank you for the feedback!</p>
<p><br>
</p>
<p>The bug with #methodArg vs. #blockArg appears to be a bit older again. With injecting, do you mean they should be simply put at [arguments at: 2] and [temporaries at: 2]? Or are you talking about to nest the current source into a pseudo template that consists
all declarations from context? In this case, I suppose we could not support #parseAMethod when context is not nil? Maybe you could clarify this :)</p>
<p><br>
</p>
<p>Regarding blocks: Hm, personally I find them intuitive to read, no matter whether the syntax is known (and the use of a hash is not that sophisticated). I think "select visible", "gather submorphs" etc. just have a clear natural language meaning. However,
I did not want to start a debate of principles, I'm sure at least one like this has been conducted before my time ... Did the community ever resolve some coding standards?</p>
<p><br>
</p>
<p>Best,</p>
<p>Christoph</p>
<div id="x_Signature">
<div name="x_divtagdefaultwrapper" style="font-family:Calibri,Arial,Helvetica,sans-serif; font-size:; margin:0">
<div><font size="2" color="#808080"></font></div>
</div>
</div>
</div>
<hr tabindex="-1" style="display:inline-block; width:98%">
<div id="x_divRplyFwdMsg" dir="ltr"><font face="Calibri, sans-serif" color="#000000" style="font-size:11pt"><b>Von:</b> Squeak-dev <squeak-dev-bounces@lists.squeakfoundation.org> im Auftrag von Levente Uzonyi <leves@caesar.elte.hu><br>
<b>Gesendet:</b> Dienstag, 1. Oktober 2019 13:14:33<br>
<b>An:</b> squeak-dev@lists.squeakfoundation.org<br>
<b>Betreff:</b> Re: [squeak-dev] The Inbox: ShoutCore-ct.73.mcz</font>
<div> </div>
</div>
</div>
<font size="2"><span style="font-size:10pt;">
<div class="PlainText">I don't think this is correct. It's clearly an improvement, but still
<br>
not perfect.<br>
Even in your test case, you wrote #methodArg instead of #blockArg, and I <br>
suppose those tests pass. But those variables are block arguments.<br>
This is because you add all the contexts' variables to the top level,<br>
which is supposed to hold the surrounding method's variables.<br>
In my opinion, the context's block structure should be injected into <br>
arguments and temporaries as they are written in that context.<br>
<br>
Also, please use blocks instead of symbols for arguments of enumeration <br>
methods (e.g. #reject:, #gather:). Using symbols is fine in scripts. They <br>
might be okay in tests, but I don't think it's appropriate to use that <br>
feature in Trunk code, no matter how comfortable it is. (I agree that the <br>
code may seem more legible when you're used to it, but what about those <br>
who are not?)<br>
<br>
Levente<br>
<br>
On Sat, 28 Sep 2019, commits@source.squeak.org wrote:<br>
<br>
> A new version of ShoutCore was added to project The Inbox:<br>
> <a href="http://source.squeak.org/inbox/ShoutCore-ct.73.mcz">http://source.squeak.org/inbox/ShoutCore-ct.73.mcz</a><br>
><br>
> ==================== Summary ====================<br>
><br>
> Name: ShoutCore-ct.73<br>
> Author: ct<br>
> Time: 28 September 2019, 11:31:09.24777 pm<br>
> UUID: 27e05259-bafa-0749-827d-b54de84bb8d2<br>
> Ancestors: ShoutCore-ct.72<br>
><br>
> Fixes a bug in context-dependent styling, as described in [1].<br>
><br>
> Until proved otherwise, we cannot distinguish between args and tempvars via DebuggerMap, so shout-reparse the original context source up to the place where the provided context starts and access the active variables from the nested shout parser.<br>
><br>
> (Minimal example to reproduce the old bug: Do it:<br>
> [:x| [:z | (x*z) halt] value: 7] value: 6<br>
> And style [z] in the context of #halt - it was wrongly styled as a tempvar before.)<br>
><br>
> [1] <a href="http://forum.world.st/Distinguishing-temp-vars-and-arguments-via-DebuggerMap-td5104614.html">
http://forum.world.st/Distinguishing-temp-vars-and-arguments-via-DebuggerMap-td5104614.html</a><br>
><br>
> =============== Diff against ShoutCore-ct.72 ===============<br>
><br>
> Item was added:<br>
> + ----- Method: SHParserST80>>activeArguments (in category 'accessing') -----<br>
> + activeArguments<br>
> + "Parsed arguments that are in the active scope"<br>
> + ^ arguments!<br>
><br>
> Item was added:<br>
> + ----- Method: SHParserST80>>activeTemporaries (in category 'accessing') -----<br>
> + activeTemporaries<br>
> + "Parsed temporaries that are in the active scope"<br>
> + ^ temporaries!<br>
><br>
> Item was changed:<br>
> ----- Method: SHParserST80>>initializeInstanceVariables (in category 'parse support') -----<br>
> initializeInstanceVariables<br>
><br>
> instanceVariables := classOrMetaClass<br>
> ifNil: [ #() ]<br>
> ifNotNil: [ classOrMetaClass allInstVarNames asArray ].<br>
> allowUnderscoreAssignments := Scanner allowUnderscoreAsAssignment.<br>
> allowUnderscoreSelectors := Scanner prefAllowUnderscoreSelectors.<br>
> allowBlockArgumentAssignment := Scanner allowBlockArgumentAssignment.<br>
> sourcePosition := 1.<br>
> arguments<br>
> ifNil: [ arguments := OrderedCollection with: nil ]<br>
> ifNotNil: [ arguments reset; addLast: nil ].<br>
> temporaries<br>
> ifNil: [ temporaries := OrderedCollection with: nil ]<br>
> ifNotNil: [ temporaries reset; addLast: nil ].<br>
> + context ifNotNil: [ self initializeVariablesFromContext ].<br>
> - context ifNotNil: [<br>
> - | contextArgumentCount contextVariableNames | <br>
> - contextArgumentCount := context numArgs.<br>
> - contextVariableNames := context tempNames asOrderedCollection.<br>
> - contextArgumentCount > 0 ifTrue: [<br>
> - arguments at: 1 put: (contextVariableNames first: contextArgumentCount) ].<br>
> - contextArgumentCount < contextVariableNames size ifTrue: [<br>
> - temporaries at: 1 put: (contextVariableNames allButFirst: contextArgumentCount) ] ].<br>
> bracketDepth := 0.<br>
> ranges<br>
> ifNil: [ ranges := OrderedCollection new: 40 "Covers over 80% of all methods." ]<br>
> ifNotNil: [ ranges reset ]!<br>
><br>
> Item was added:<br>
> + ----- Method: SHParserST80>>initializeVariablesFromContext (in category 'parse support') -----<br>
> + initializeVariablesFromContext<br>
> + <br>
> + | contextSourcePcIndex contextSourceParser |<br>
> + contextSourcePcIndex := (context debuggerMap<br>
> + rangeForPC: context pc<br>
> + in: context method<br>
> + contextIsActiveContext: true "little white lie to work in every situation")<br>
> + start.<br>
> + contextSourceParser := self class new<br>
> + classOrMetaClass: context method methodClass;<br>
> + environment: self environment;<br>
> + source: (context method getSource first: contextSourcePcIndex);<br>
> + yourself.<br>
> + contextSourceParser parse.<br>
> + arguments at: 1 put: ((contextSourceParser activeArguments<br>
> + reject: #isNil) gather: #yourself) asOrderedCollection.<br>
> + temporaries at: 1 put: ((contextSourceParser activeTemporaries <br>
> + reject: #isNil) gather: #yourself) asOrderedCollection.!<br>
<br>
</div>
</span></font>
</body>
</html>