Refactoring Inspectors

Hi all! :-)

After a long, long time of refactoring and being busy with other projects, I'm excited to finally present you a second version of the refactored & completely refurbished inspectors! For the traceback: The first version I created last summer lead to Tools-ct.900 which you gave me many helpful reviews for. Thanks to Marcel and Chris for all the time they spent in looking at the changes! Attached you find the files that form the second refactoring.

Details on how to load these into your image:

This time I decided against sending the proposals into the inbox, as, unfortunately, many packages are coupled to the inspector, such as Protocols, EToys, or Morphic (even though only one reference). It was thus easier to create a changeset. Due to the scriptless nature of changesets, please do the following:

1. File in Inspector-prerequisites.cs. This is a subset of Collections-ct.875 which must be loaded in two steps because the involved method #indexOf: is called during file in progress.
2. File in Inspector.cs. Confirm to create the two class pools automatically, sigh ...
3. Only in case you had any inspectors opened, you might get some nil errors. In this case, run Inspector-postload.st and then restart the erroneous processes from the second context to fix these errors (if this should ever get into Trunk, we can automate this, of course, without displaying any errors to the user).

Finally syntax errors inside the inspector pane:

Custom fields:

Shout styling of the field titles:

Dragging (custom) fields from one inspector to another (experimental):

self systemNavigation

browseMessageList: [ (PackageInfo named: 'Tools') methods select: [:method |

({method actualClass category. method actualClass whichCategoryIncludesSelector: method selector} anySatisfy: [:cat | '*inspect*' match: cat asLowercase])

and: [#(breakpoint flag) includes: (method actualClass toolIconSelector: method selector)]]]

name: 'Flags in Inspector'.

Am 16.02.2020 23:13:41 schrieb Thiede, Christoph <[hidden email]>:

Hi all! :-)

After a long, long time of refactoring and being busy with other projects, I'm excited to finally present you a second version of the refactored & completely refurbished inspectors! For the traceback: The first version I created last summer lead to Tools-ct.900 which you gave me many helpful reviews for. Thanks to Marcel and Chris for all the time they spent in looking at the changes! Attached you find the files that form the second refactoring.

Details on how to load these into your image:

This time I decided against sending the proposals into the inbox, as, unfortunately, many packages are coupled to the inspector, such as Protocols, EToys, or Morphic (even though only one reference). It was thus easier to create a changeset. Due to the scriptless nature of changesets, please do the following:

1. File in Inspector-prerequisites.cs. This is a subset of Collections-ct.875 which must be loaded in two steps because the involved method #indexOf: is called during file in progress.
2. File in Inspector.cs. Confirm to create the two class pools automatically, sigh ...
3. Only in case you had any inspectors opened, you might get some nil errors. In this case, run Inspector-postload.st and then restart the erroneous processes from the second context to fix these errors (if this should ever get into Trunk, we can automate this, of course, without displaying any errors to the user).

Overview of the changes made against Tools-ct.900:

My original motivation for this refactoring was to eliminate the large number of hard-coded references to indices in the field list and make all the selection logic more reusable. This was mainly done in Tools-ct.900.

The primary suggestion I was told then was not to use a cryptic OrderedDictionary with case-dependent value meanings for describing the inspector fields, but rather a spec class. This was a very great idea, so now each field in an inspector is represented by an instance of InspectorField. IMHO this makes management and overall communication between an inspector and its component much easier. As an extra side effect after that change, it was super easy to implement custom fields, which can be created live by the user to observe and manipulate a non-trivial aspect of the inspectee. Here are some impressions:

Finally syntax errors inside the inspector pane:

Custom fields:

Shout styling of the field titles:

Dragging (custom) fields from one inspector to another (experimental):

Furthermore, I added a test suite for all inspector classes in the Trunk.

Here is a detailed changelog (still against Tools-ct.900, so to say a diff-diff):

• Introduce InspectorField (OOP ❤️)
• Add support for custom fields
• Loads of refactoring, in order to beautify and deduplicate inspector classes, and to make it easier to create new inspector subclasses:
  • Introduce streaming pattern for building the field list
  • Deduplicate + merge inspector operations on collections (CollectionInspector), allow modifying all kinds of collections
  • Use ContextInspector by default for inspecting contexts
  • Deduplicate + merge menus, make all promised shortcuts work everywhere
• New inspector subclass: ClassInspector
• Deprecate several obsoleted selectors
  • drop support for #defaultIntegerBaseInDebugger
• Revise constructors – #openOn: does not use #inspectorClass, but #inspect: does

• Don’t abuse #initialize:

  • now, 

    #object: exchanges the inspectee
  • #inspect: may also exchange the inspector
• Improve documentation
  
• Improve error detection
  
• Revise stepping/updating mechanism
  
  • #update now updates field list as well (useful for observing a collection, for example)
  • Please report any performance issues!

• Automatically restore latest selected slot name, use #codeChangedElsewhere

  • Open Problem: How can we distinguish between both text fields?

  • Disable removal of contents that were typed into debugger's inspectors via #restore...InspectorState (*)
    

• Recategorize extension methods
  
• Use Shout styler for displaying field list, also respect UI theme changes
• Improve multilingual support
• Move #inspectElement as extension method into Object
• Allow modification of CompiledMethods
• Revise InspectorBrowser: Treat Inspector & Browser equally and use decomposition instead of subclassing
• Adds a test suite for all kinds of inspectors
• Feature: exchange inspectee by accepting an object in the #self slot
• chasePointers and explorePointers are no longer miss-overridden in Inspector but directly called on selectionOrObject
• Debuggers respect #inspectorClass for the receiver inspector

Open todos/questions/topics of discussion:

Oh, just do the following:

self systemNavigation

browseMessageList: [ (PackageInfo named: 'Tools') methods select: [:method |

({method actualClass category. method actualClass whichCategoryIncludesSelector: method selector} anySatisfy: [:cat | '*inspect*' match: cat asLowercase])

and: [#(breakpoint flag) includes: (method actualClass toolIconSelector: method selector)]]]

name: 'Flags in Inspector'.

:-)

Further questions:

• (*) Can we find any better solution for #restore...InspectorState? Personally, I find it kind of annoying when I enter something into a debugger inspector, switch to another context and loose that command for the moment. Not to be confused with the selectedFieldName memory, which is very useful. I would rather like to revert this particular change. Or alternatively, to make a preference for it :-)
• The current solution for inspector text fields uses #askBeforeDiscardingEdits: which is safer than before, but it asks too often when changing the selection. How can we fix this?
• A possible design issue: At the moment, InspectorField does not know about its inspector, it's extrinsic for all relevant operations. This keeps them a bit more decoupled, and makes it possible to use the same field for multiple inspectors (you can try this out via drag'n'drop while holding shift). On the other hand, one might argue that the number of parametrized messages in InspectorField might be too high. Unless you give me some new perspectives, I would tend to keep the current design.

What's next?

I will appreciate every single feedback of y'all. I really depend on it! Just file in the changeset and tell me any potential problems. For a detailed code review, you can simply send me a follow-up changeset. We can also set up a git repository or so if you think this scales better. I'm looking forward to your review!

Best,

Christoph

@all Here is an updated changeset that can be loaded stand-alone. Please try! :-)

Marcel, thank you for the pointer! This is exactly what I was searching for.

Please note that in order to regain full functionality, after file in you will need to reopen all inspectors or debuggers because I changed some toolbuilder selectors (accepting something in the value pane). Would this also be worth a postscript?

Best,

Christoph

---

Von: Taeumel, Marcel
Gesendet: Montag, 17. Februar 2020 10:36 Uhr
An: Thiede, Christoph; Javier Diaz-Reinoso via Squeak-dev
Betreff: Re: Refactoring Inspectors

 

Hi Christoph.

ChangeSets can have both preamble and postscripts:

Best,

Marcel

Am 16.02.2020 23:13:41 schrieb Thiede, Christoph <[hidden email]>:

Hi all! :-)

After a long, long time of refactoring and being busy with other projects, I'm excited to finally present you a second version of the refactored & completely refurbished inspectors! For the traceback: The first version I created last summer lead to Tools-ct.900 which you gave me many helpful reviews for. Thanks to Marcel and Chris for all the time they spent in looking at the changes! Attached you find the files that form the second refactoring.

Details on how to load these into your image:

This time I decided against sending the proposals into the inbox, as, unfortunately, many packages are coupled to the inspector, such as Protocols, EToys, or Morphic (even though only one reference). It was thus easier to create a changeset. Due to the scriptless nature of changesets, please do the following:

1. File in Inspector-prerequisites.cs. This is a subset of Collections-ct.875 which must be loaded in two steps because the involved method #indexOf: is called during file in progress.
2. File in Inspector.cs. Confirm to create the two class pools automatically, sigh ...
3. Only in case you had any inspectors opened, you might get some nil errors. In this case, run Inspector-postload.st and then restart the erroneous processes from the second context to fix these errors (if this should ever get into Trunk, we can automate this, of course, without displaying any errors to the user).

Overview of the changes made against Tools-ct.900:

My original motivation for this refactoring was to eliminate the large number of hard-coded references to indices in the field list and make all the selection logic more reusable. This was mainly done in Tools-ct.900.

The primary suggestion I was told then was not to use a cryptic OrderedDictionary with case-dependent value meanings for describing the inspector fields, but rather a spec class. This was a very great idea, so now each field in an inspector is represented by an instance of InspectorField. IMHO this makes management and overall communication between an inspector and its component much easier. As an extra side effect after that change, it was super easy to implement custom fields, which can be created live by the user to observe and manipulate a non-trivial aspect of the inspectee. Here are some impressions:

Finally syntax errors inside the inspector pane:

Custom fields:

Shout styling of the field titles:

Dragging (custom) fields from one inspector to another (experimental):

Furthermore, I added a test suite for all inspector classes in the Trunk.

Here is a detailed changelog (still against Tools-ct.900, so to say a diff-diff):

• Introduce InspectorField (OOP ❤️)
• Add support for custom fields
• Loads of refactoring, in order to beautify and deduplicate inspector classes, and to make it easier to create new inspector subclasses:
  • Introduce streaming pattern for building the field list
  • Deduplicate + merge inspector operations on collections (CollectionInspector), allow modifying all kinds of collections
  • Use ContextInspector by default for inspecting contexts
  • Deduplicate + merge menus, make all promised shortcuts work everywhere
• New inspector subclass: ClassInspector
• Deprecate several obsoleted selectors
  • drop support for #defaultIntegerBaseInDebugger
• Revise constructors – #openOn: does not use #inspectorClass, but #inspect: does

• Don’t abuse #initialize:

  • now, 

    #object: exchanges the inspectee
  • #inspect: may also exchange the inspector
• Improve documentation
  
• Improve error detection
  
• Revise stepping/updating mechanism
  
  • #update now updates field list as well (useful for observing a collection, for example)
  • Please report any performance issues!

• Automatically restore latest selected slot name, use #codeChangedElsewhere

  • Open Problem: How can we distinguish between both text fields?

  • Disable removal of contents that were typed into debugger's inspectors via #restore...InspectorState (*)
    

• Recategorize extension methods
  
• Use Shout styler for displaying field list, also respect UI theme changes
• Improve multilingual support
• Move #inspectElement as extension method into Object
• Allow modification of CompiledMethods
• Revise InspectorBrowser: Treat Inspector & Browser equally and use decomposition instead of subclassing
• Adds a test suite for all kinds of inspectors
• Feature: exchange inspectee by accepting an object in the #self slot
• chasePointers and explorePointers are no longer miss-overridden in Inspector but directly called on selectionOrObject
• Debuggers respect #inspectorClass for the receiver inspector

Open todos/questions/topics of discussion:

Oh, just do the following:

self systemNavigation

browseMessageList: [ (PackageInfo named: 'Tools') methods select: [:method |

({method actualClass category. method actualClass whichCategoryIncludesSelector: method selector} anySatisfy: [:cat | '*inspect*' match: cat asLowercase])

and: [#(breakpoint flag) includes: (method actualClass toolIconSelector: method selector)]]]

name: 'Flags in Inspector'.

:-)

Further questions:

• (*) Can we find any better solution for #restore...InspectorState? Personally, I find it kind of annoying when I enter something into a debugger inspector, switch to another context and loose that command for the moment. Not to be confused with the selectedFieldName memory, which is very useful. I would rather like to revert this particular change. Or alternatively, to make a preference for it :-)
• The current solution for inspector text fields uses #askBeforeDiscardingEdits: which is safer than before, but it asks too often when changing the selection. How can we fix this?
• A possible design issue: At the moment, InspectorField does not know about its inspector, it's extrinsic for all relevant operations. This keeps them a bit more decoupled, and makes it possible to use the same field for multiple inspectors (you can try this out via drag'n'drop while holding shift). On the other hand, one might argue that the number of parametrized messages in InspectorField might be too high. Unless you give me some new perspectives, I would tend to keep the current design.

What's next?

I will appreciate every single feedback of y'all. I really depend on it! Just file in the changeset and tell me any potential problems. For a detailed code review, you can simply send me a follow-up changeset. We can also set up a git repository or so if you think this scales better. I'm looking forward to your review!

Best,

Christoph