Eliot Miranda uploaded a new version of Morphic to project The Trunk:
http://source.squeak.org/trunk/Morphic-eem.1742.mcz ==================== Summary ==================== Name: Morphic-eem.1742 Author: eem Time: 12 April 2021, 11:32:16.715075 am UUID: c88153a2-36a5-447f-abd0-93a6b164fd6f Ancestors: Morphic-mt.1741 Eliminate shadowed variable warning(s). =============== Diff against Morphic-mt.1741 =============== Item was changed: ----- Method: BottomLeftGripMorph>>apply: (in category 'target resize') ----- apply: delta | oldBounds | oldBounds := self target bounds. self target bounds: (oldBounds origin + (delta x @ 0) corner: oldBounds corner + (0 @ delta y)). self flag: #workaround. "mt: Due to a layout-specific 'let us start in the top-left corner of a layout cell'-behavior, we have to go up the owner chain and propagate the delta. See Morph >> #layoutInBounds:positioning: and there section 1.2." + self target allOwnersDo: + [:anOwner | + (anOwner layoutPolicy notNil and: [anOwner ~~ Project current world]) + ifTrue: [anOwner left: owner left + delta x]].! - self target allOwnersDo: [:owner | - (owner layoutPolicy notNil and: [owner ~~ Project current world]) - ifTrue: [owner left: owner left + delta x]].! Item was changed: ----- Method: LeftGripMorph>>apply: (in category 'target resize') ----- apply: delta | oldBounds | oldBounds := self target bounds. self target bounds: (oldBounds origin + (delta x @ 0) corner: oldBounds corner). self flag: #workaround. "mt: Due to a layout-specific 'let us start in the top-left corner of a layout cell'-behavior, we have to go up the owner chain and propagate the delta. See Morph >> #layoutInBounds:positioning: and there section 1.2." + self target allOwnersDo: + [:anOwner | + (anOwner layoutPolicy notNil and: [anOwner ~~ Project current world]) + ifTrue: [anOwner left: anOwner left + delta x]].! - self target allOwnersDo: [:owner | - (owner layoutPolicy notNil and: [owner ~~ Project current world]) - ifTrue: [owner left: owner left + delta x]].! Item was changed: ----- Method: TopGripMorph>>apply: (in category 'target resize') ----- apply: delta | oldBounds | oldBounds := self target bounds. self target bounds: (oldBounds origin + (0 @ delta y) corner: oldBounds corner). self flag: #workaround. "mt: Due to a layout-specific 'let us start in the top-left corner of a layout cell'-behavior, we have to go up the owner chain and propagate the delta. See Morph >> #layoutInBounds:positioning: and there section 1.2." + self target allOwnersDo: + [:anOwner | + (anOwner layoutPolicy notNil and: [anOwner ~~ Project current world]) + ifTrue: [anOwner top: anOwner top + delta y]].! - self target allOwnersDo: [:owner | - (owner layoutPolicy notNil and: [owner ~~ Project current world]) - ifTrue: [owner top: owner top + delta y]].! Item was changed: ----- Method: TopLeftGripMorph>>apply: (in category 'target resize') ----- apply: delta | oldBounds | oldBounds := self target bounds. self target bounds: (oldBounds origin + delta corner: oldBounds corner). self flag: #workaround. "mt: Due to a layout-specific 'let us start in the top-left corner of a layout cell'-behavior, we have to go up the owner chain and propagate the delta. See Morph >> #layoutInBounds:positioning: and there section 1.2." + self target allOwnersDo: + [:anOwner | + (anOwner layoutPolicy notNil and: [anOwner ~~ Project current world]) + ifTrue: [anOwner topLeft: anOwner topLeft + delta]].! - self target allOwnersDo: [:owner | - (owner layoutPolicy notNil and: [owner ~~ Project current world]) - ifTrue: [owner topLeft: owner topLeft + delta]].! Item was changed: ----- Method: TopRightGripMorph>>apply: (in category 'target resize') ----- apply: delta | oldBounds | oldBounds := self target bounds. self target bounds: (oldBounds origin + (0@delta y) corner: oldBounds corner + (delta x @ 0)). self flag: #workaround. "mt: Due to a layout-specific 'let us start in the top-left corner of a layout cell'-behavior, we have to go up the owner chain and propagate the delta. See Morph >> #layoutInBounds:positioning: and there section 1.2." + self target allOwnersDo: + [:anOwner | + (anOwner layoutPolicy notNil and: [anOwner ~~ Project current world]) + ifTrue: [anOwner top: anOwner top + delta y]].! - self target allOwnersDo: [:owner | - (owner layoutPolicy notNil and: [owner ~~ Project current world]) - ifTrue: [owner top: owner top + delta y]].! |
Hi Eliot,
I fear that changes like this will not be very durable unless tested in any way. At the moment, you won't even notice a new shadow unless you have opened a Transcript by accident when accepting a method. Should we maybe show a message window for shadows instead when compiled interactively, analogously to the warnings about superfluous temps? (In a larger context, I would strongly opt for (finally) introducing a mechanism for linter/compiler annotations in CodeHolders. Most modern editors and IDEs have it, even Pharo; if Squeak does not want to get left behind, we should support something like this, too. It could or should be configurable, of course.) LBNL, if I understand Chris correctly, "anOwner" might be a suboptimal name for a variable that holds Morph instances, right? :) Best, Christoph ----- Carpe Squeak! -- Sent from: http://forum.world.st/Squeak-Dev-f45488.html
Carpe Squeak!
|
Hi all! Hehe. "shadows + morphic" got me distracted. ^__^ Hmm... personally, I prefer "owner" over "anOwner" as block variable. That is, I don't care about shadowing the instVar name -- just locally focused readability. That is, I am a fan of accessors and thus rarely (? never ?) bitten by confusing instVar access and tempVar access. :-) For classes with longer methods and extensive instVar access (...Compiler?), shadowing can be an issue for code readability, though. > In a larger context, I would strongly opt for (finally) introducing a > mechanism for linter/compiler annotations in CodeHolders. Most modern > editors and IDEs have it, even Pharo; if Squeak does not want to get left > behind, we should support something like this, too. It could or should be > configurable, of course. How come? While structured information is nice, I am opposed to introducing more things that are prone to errors. :-) What would you expect me to annotate in such a case? Marking intentional shadowing? I wouldn't want to. ;-) Tool-specific configuration/annotation -- yes, linter and compiler are tools -- should not interfere with an artifact's primary representation. For example, it's problematic that there is only a single #inspectorClass or #explorerContents per domain object. That does not scale; it can also obfuscate the object's interface. In a similar manner, if one would start to "enrich" source code with various annotations for all kinds of supported compilers/parsers/linters etc. ... nah. There are already pragmas, which can be used to annotate methods. Sub-method annotation might be beneficial in not-too-frequent situations ... thus, maybe not worth the effort. Best, Marcel
|
Hi Marcel,
> That is, I don't care about shadowing the instVar name -- just locally > focused readability. That is, I am a fan of accessors and thus rarely (? > never ?) bitten by confusing instVar access and tempVar access. :-) For > classes with longer methods and extensive instVar access (...Compiler?), > shadowing can be an issue for code readability, though. That is an interesting insight. Personally, I have stumbled upon unintended shadows a number of times, which is probably related to my way of dealing with tempvars - usually, I do not declare any of them manually but have them added by the compiler when accepting the method. With this approach, it can happen quickly that you miss a tempvar declaration because a variable with the same name already exists. Analogously sometimes, I remove all declarations after refactoring a method when I am unsure whether all tempvars are still in use - and in this situation, I would be irritated if no tempvar declaration is generated because a prior author of the method (most likely a stupid former self) has shadowed an instvar. But as it depends on the individual coding style, as you write, do you think we should provide a toggle/preference to decriminalize instvar shadows? *** > > linter/compiler annotations ... > How come? Well, there might be a lot of opinions on this issue and probably I did not express my perspective in the right way. Please give me a second chance. :-) Just to make sure what we are talking about - when I said annotations, I meant something like this: <http://web.archive.org/web/20200504234326/http://www.peachpie.io/wp-content/uploads/2017/02/syntax-error.png> That is, visual highlight + additional information that is shown upon interaction (hover or click). Are we talking about the same kind of annotations? :-) Such annotations could be used for different things: Compiler warnings (undeclared/undefined/unused temp, shadowed variable, deprecated syntax such as underscore assignments), syntax errors (if we manage to make the Compiler more robust), linter warnings (for example SwaLint), spelling errors, etc. At the moment, we communicate all this information in several different ways: Some (syntax errors) are displayed via Shout, which is nice but lacks proper self-explanation ("Why is this all red? What is wrong with this?") (and also requires us to maintain two separate parsers); others (compiler warnings) are shown only when a method is accepted, so here we have a chance to shorten feedback loops and make the compilation experience more live; even others are shown in the Transcript only, which only a few people will notice at all; and some others will just not be shown unless you use another tool such as the SwaLint UI to see these messages. A common solution that we can see in many modern tools is interactive (sometimes expandable/explorable) annotations that are fed by multiple sources. A good example (maybe even the gold standard?) might be the language server API in Visual Studio Code. I think the popularity of this concept is just based on its usefulness: You do not need to check multiple tools but can union all their outputs in a single view (sink), combined with the ability to filter different sources individually. > While structured information is nice, I am opposed to introducing more > things that are prone to errors. :-) What kind of errors do you mean exactly? If robustness is a thing, I would not give up the entire concept instead of using a few #ifError's. :-) > yes, linter and compiler are tools Hmm ... when I work with the source code pane of a CodeHolder, I actually have the impression to work with the compiler itself, at least I can't name any other tool that would be more close to writing code. See interactive compilation. The compiler is a necessary step in the pipeline of making the entered code executable. > Tool-specific configuration/annotation [...] should not interfere with an > artifact's primary representation. [...] That does not scale; it can also > obfuscate the object's interface. I think I see your point, too many annotations could be distracting. But I am confident that we could overcome this problem by adding a simple but powerful filter mechanism. And very of course, these annotations should be configurable using preferences. Besides, at the moment, there would be only two or three kinds of annotation sources at all, so this might also be a problem for the not-so-near future. :-) > For example, it's problematic that there is only a single #inspectorClass > or #explorerContents per domain object. > various annotations for all kinds of supported compilers/parsers/linters > etc. ... nah. Yeah, we could add another indirection for dispatching access to such tools. But I do not see how this would restrict our abilities to filter this (hypothetical) mass of tools? Also, in the case of compilers, we already have #compilerClass. In the context of a CodeHolder, you would need to decide on one of many compilers anyway ... > There are already pragmas, which can be used to annotate methods. That confuses me. :-) This is a completely different kind of annotations, isn't it? Pragmas are used to store additional, intrinsic information attached to the source code, hover annotations are used to display derived information from different tools - they are transient but not a part of the primary object itself. My proposal was *not* to insert any durable artifacts/comments into the source code while analyzing it. And perhaps one last, more general thought: I think I don't have to emphasize that I'm a big fan of the Squeak tools like most people who will read this. :-) Nevertheless, I have to admit that many other (I call them "modern") editors/IDEs are based on architectures that appear to be more flexible, more scalable, more composed than our CodeHolder and our Browser are. I'm thinking about Visual Studio and Visual Studio Code when making this statement. They provide great opportunities for third-party tools to integrate potentially useful enhancements into the existing IDEs. I don't like to say it, but my impression is that their approach would be more suitable than our tooling which has grown over time when you try to customize the entire development system for a specific domain. Think of exciting additions such as Babylonian programming or also Fabio's new CallTargetBrowser. It's a pity that you still need to subclass from the main tool class (Browser) in all these situations to install such small but useful extensions. VS Code, on the other hand, has a more extensible design and provides lots of APIs that make the development of a small utility more simple. That being said, I don't want to say that the Squeak/Smalltalk approach of designing tools is wrong and should be abandoned - the opposite is true, the liveness and self-description of Squeak are still unmatched! But I think that we need to make more efforts to modularize the Squeak tools and provide more extension points. The new InspectorField API might be a good step in this direction, but we need much more of these APIs. It would be a shame if developers preferred a system like VS Code over Squeak/Smalltalk because it's easier to extend - in particular in the light that Squeak is frequently used for research and experiments. That's why I said in my previous message, "If Squeak does not want to get left behind, we should support something like this [annotations]". :-) Well, I am glad that you survived this massive wall of text and I'm looking forward to your thoughts. :-) Best, Christoph ----- Carpe Squeak! -- Sent from: http://forum.world.st/Squeak-Dev-f45488.html
Carpe Squeak!
|
In reply to this post by Christoph Thiede
Hi Christoph,
> On Apr 12, 2021, at 12:32 PM, Christoph Thiede <[hidden email]> wrote: > > Hi Eliot, > > I fear that changes like this will not be very durable unless tested in any > way. At the moment, you won't even notice a new shadow unless you have > opened a Transcript by accident when accepting a method. Should we maybe > show a message window for shadows instead when compiled interactively, > analogously to the warnings about superfluous temps? I’m removing shadows, not adding new ones. In all cars I saw eleven ing s from the compiler in the update stream and replaced shadowed temp cars by renamed temp vars. So my changes have no semantic effect and only remove the warning. > > (In a larger context, I would strongly opt for (finally) introducing a > mechanism for linter/compiler annotations in CodeHolders. Most modern > editors and IDEs have it, even Pharo; if Squeak does not want to get left > behind, we should support something like this, too. It could or should be > configurable, of course.) > > LBNL, if I understand Chris correctly, "anOwner" might be a suboptimal name > for a variable that holds Morph instances, right? :) > > Best, > Christoph > > > > ----- > Carpe Squeak! > -- > Sent from: http://forum.world.st/Squeak-Dev-f45488.html > |
Hi Christoph, hi Eliot, it might be that the likelihood of shadowing increases with the use of cull'ed block invocations, which we have been supporting more and more in the last years: self owner ifNotNil: [:owner | "..."]. self submorphs ifNotEmpty: [:submorphs | "..."]. As alternative names, one could drop their actual "role" and just use their kinds (as generic as desired): self owner ifNotNil: [:morph | "..."]. self submorphs ifNotEmpty: [:morphs | "..."]. self submorphs ifNotEmpty: [:objects | "..."]. In the past, I happened to mistakenly have used the wrong role, shadowing the right one: self allOwnersDo: [:owner | "..."]. self allOwnersDo: [:morph | "..."]. Yet, we do have misleading protocol on what to actually call "owner." Just the morph that has me directly in its submorphs? That might be another topic. :-) *** To avoid shadowing, I find myself using (a) abbreviations, (b) kind/class, or (c) count-prefix "a" or "some". Or a combination of those. Unfortunately, omitting the domain-specific role in a name might impair readability. I just noticed that I am finde with "aMorph" as a method argument, yet I would never use "a" in a block argument. :-D Strange. Best, Marcel
|
Hi all,
Slight tangent - I wanted to show a place where I actively want to be able to shadow variables lexically. I am working with an Actor DSL that expands to use of a "well-known" local variable called "thisTurn". Here's an example of the kind of expanded code I want to be able to have: thisTurn spawn: [:thisTurn | "NB. thisTurn here is in a different Actor's context to the thisTurn that had #spawn: sent to it!" thisTurn assert: (Presence new userName: 'me'). thisTurn assert: (Observe new label: #Presence; observer: (thisTurn ref onAsserted: [:thisTurn :p :handle | "Similarly, thisTurn here is the turn active at the time an asserted event matching the given label is processed, not the time that the Observe assertion is made." "..."]; onRetracted: [:thisTurn :handle | "..."]))] The idea is that thisTurn should represent the ambiently-active-and-current Turn at the time each little fragment of code runs. I could use a DynamicVariable, but it seems a bit of a shame to use such a big hammer to (poorly?) simulate what lexical scope gives for free. TL;DR: To this old functional programmer, shadowing within a method isn't an error, but is instead useful. Now that we have had proper lexical closures for quite a few years, could we consider lifting the restriction on shadowing? I'm aware there would likely be social obstacles to doing so; would there also be technical obstacles? Cheers, Tony On 4/18/21 2:44 PM, Marcel Taeumel wrote: > Hi Christoph, hi Eliot, > > it might be that the likelihood of shadowing increases with the use of > cull'ed block invocations, which we have been supporting more and more > in the last years: > > self owner ifNotNil: [:owner | "..."]. > self submorphs ifNotEmpty: [:submorphs | "..."]. > > As alternative names, one could drop their actual "role" and just use > their kinds (as generic as desired): > > self owner ifNotNil: [:morph | "..."]. > self submorphs ifNotEmpty: [:morphs | "..."]. > self submorphs ifNotEmpty: [:objects | "..."]. > > In the past, I happened to mistakenly have used the wrong role, > shadowing the right one: > > self allOwnersDo: [:owner | "..."]. > self allOwnersDo: [:morph | "..."]. > > Yet, we do have misleading protocol on what to actually call "owner." > Just the morph that has me directly in its submorphs? That might be > another topic. :-) > > *** > > To avoid shadowing, I find myself using (a) abbreviations, (b) > kind/class, or (c) count-prefix "a" or "some". Or a combination of > those. Unfortunately, omitting the domain-specific role in a name might > impair readability. > > I just noticed that I am finde with "aMorph" as a method argument, yet I > would never use "a" in a block argument. :-D Strange. > > Best, > Marcel > >> Am 18.04.2021 05:39:33 schrieb Eliot Miranda <[hidden email]>: >> >> Hi Christoph, >> >> >> > On Apr 12, 2021, at 12:32 PM, Christoph Thiede wrote: >> > >> > Hi Eliot, >> > >> > I fear that changes like this will not be very durable unless tested >> in any >> > way. At the moment, you won't even notice a new shadow unless you have >> > opened a Transcript by accident when accepting a method. Should we maybe >> > show a message window for shadows instead when compiled interactively, >> > analogously to the warnings about superfluous temps? >> >> I’m removing shadows, not adding new ones. In all cars I saw eleven >> ing s from the compiler in the update stream and replaced shadowed >> temp cars by renamed temp vars. So my changes have no semantic effect >> and only remove the warning. >> >> > >> > (In a larger context, I would strongly opt for (finally) introducing a >> > mechanism for linter/compiler annotations in CodeHolders. Most modern >> > editors and IDEs have it, even Pharo; if Squeak does not want to get >> left >> > behind, we should support something like this, too. It could or >> should be >> > configurable, of course.) >> > >> > LBNL, if I understand Chris correctly, "anOwner" might be a >> suboptimal name >> > for a variable that holds Morph instances, right? :) >> > >> > Best, >> > Christoph >> > >> > >> > >> > ----- >> > Carpe Squeak! >> > -- >> > Sent from: http://forum.world.st/Squeak-Dev-f45488.html >> > >> > > |
Hi Tony, thanks for this example. :-) > I could use a DynamicVariable, but it seems a bit of a shame to use such > a big hammer to (poorly?) simulate what lexical scope gives for free.
Hmm.... I wonder if that would be actually a "big hammer" or just an indication that you favor larger pieces of code (i.e. long methods) over short ones. In modules composed of many short(-ish) methods, I tend to focus on object messaging and higher-level control flow. There, I do not think about lexical scoping as an important design element. In Vivide, I use DynamicVariable to scope relevant domain objects (i.e. active vivide, active profile, active organization, active script, ...). :-) (I noticed that Newspeak puts a stronger focus on lexical scoping than Squeak/Smalltalk does.) Best, Marcel
|
On 4/19/21 10:47 AM, Marcel Taeumel wrote:
> Hmm.... I wonder if that would be actually a "big hammer" or just an > indication that you favor larger pieces of code (i.e. long methods) over > short ones. In modules composed of many short(-ish) methods, I tend to > focus on object messaging and higher-level control flow. There, I do not > think about lexical scoping as an important design element. Yeah. Note that this code is the output of a code generator that avoids the need for most user-visible explicit references to thisTurn. User code would be shorter and simpler, something like spawn: [ assert: (Presence new userName: 'me'). assert: (Observe new label: #Presence; observer: ( onAsserted: [:p :handle | ...]; onRetracted: [:handle | ...]))] And it's still an open (research) question as to whether Smalltalk is well-suited to this style of programming at all. Overuse of blocks (especially blocks as state, i.e. callbacks!) makes for poor integration with tooling and bad interactions with edits to methods. In a parallel track I'm trying to figure out a way of avoiding blocks and using methods more (that is, trying to find a different way of expressing my DSL that fits better with Smalltalk as-is). I'd still want to hide thisTurn where possible though, just like thisContext is hidden. > In Vivide, I use DynamicVariable to scope relevant domain objects (i.e. > active vivide, active profile, active organization, active script, ...). :-) Ah! Thanks, I'll check that out, see if I can take inspiration from it! Regards, Tony |
In reply to this post by marcel.taeumel
Hi Eliot, hi Marcel,
> I’m removing shadows, not adding new ones.
I do not doubt this. I only wanted to point to the fact that your refactoring is completely free of safety. If someone else says, "Hey, these names are too long, let's shorten them", then the shadows might be introduced again and no one will notice. A change in tooling could prevent this problem; otherwise, automated tests would do, too. Just a notice. :-)
> I just noticed that I am finde with "aMorph" as a method argument, yet I would never use "a" in a block argument. :-D Strange. +1. I also use roles as names (but I do not really like it - these are not really explaining names). Abbreviations are counter-intuitive, too. Another option is "ea", but this only good for very small methods, if at all. I use "some" analogously to "a"
for method parameters that should be collections. Maybe "the" might be a better pattern for naming block args? :-)
> self owner ifNotNil: [:theOwner | "..."].
> self submorphs ifNotEmpty: [:theSubmorphs | "..."].
Best,
Christoph
Von: Squeak-dev <[hidden email]> im Auftrag von Taeumel, Marcel
Gesendet: Sonntag, 18. April 2021 14:44:32 An: squeak-dev Betreff: Re: [squeak-dev] Shadows (was: The Trunk: Morphic-eem.1742.mcz)
Hi Christoph, hi Eliot,
it might be that the likelihood of shadowing increases with the use of cull'ed block invocations, which we have been supporting more and more in the last years:
self owner ifNotNil: [:owner | "..."].
self submorphs ifNotEmpty: [:submorphs | "..."].
As alternative names, one could drop their actual "role" and just use their kinds (as generic as desired):
self owner ifNotNil: [:morph | "..."].
self submorphs ifNotEmpty: [:morphs | "..."].
self submorphs ifNotEmpty: [:objects | "..."].
In the past, I happened to mistakenly have used the wrong role, shadowing the right one:
self allOwnersDo: [:owner | "..."].
self allOwnersDo: [:morph | "..."].
Yet, we do have misleading protocol on what to actually call "owner." Just the morph that has me directly in its submorphs? That might be another topic. :-)
***
To avoid shadowing, I find myself using (a) abbreviations, (b) kind/class, or (c) count-prefix "a" or "some". Or a combination of those. Unfortunately, omitting the domain-specific role in a name might impair readability.
I just noticed that I am finde with "aMorph" as a method argument, yet I would never use "a" in a block argument. :-D Strange.
Best,
Marcel
Carpe Squeak!
|
In this particular case, I would suggest myOwner, mySubmorphs. Am Mo., 19. Apr. 2021 um 19:43 Uhr schrieb Thiede, Christoph <[hidden email]>:
|
Also possible. :-) Whereas personally, I associate the "my" prefix with demo and example code but not production. :-) Best,
Christoph
Von: Squeak-dev <[hidden email]> im Auftrag von Jakob Reschke <jakres+[hidden email]>
Gesendet: Montag, 19. April 2021 19:46:40 An: The general-purpose Squeak developers list Betreff: Re: [squeak-dev] Shadows (was: The Trunk: Morphic-eem.1742.mcz) In this particular case, I would suggest myOwner, mySubmorphs.
Am Mo., 19. Apr. 2021 um 19:43 Uhr schrieb Thiede, Christoph <[hidden email]>:
Carpe Squeak!
|
No great difference there: if your demo or example code were an instance, myThing might also have been the result of `self thing`. ;-) Am Mo., 19. Apr. 2021 um 19:47 Uhr schrieb Thiede, Christoph <[hidden email]>:
|
Free forum by Nabble | Edit this page |