On Nov 24, 2006, at 1:10 PM, Andreas Raab wrote:
Brian Rice wrote:
So, in this builder design pattern, you construct a tree of specification objects, pass it to the Builder object and tell it to build: them. This seems rather Java-inspired, but that's beside the point.
That's a funny comment given how little exposure I had to Java ;-)
Fair enough. It's just my reaction to "Builder build:" idioms where the Builder class is only there for that build: protocol and is stateless (okay, there's a #parent slot, but that wouldn't conflict with any other protocol except if the name clashed). It's "nouning a verb", if you will. Usually that means it's fine to move the protocol to another class; the concrete one that comes to mind is the UIManager, which notably already also is subclassed for each UI framework. In fact, I'd like to make that a concrete suggestion, tempered by the political expense of getting users of the protocol to switch (but that seems like one class reference per user, so probably not difficult).
To continue this mental experiment a little, I'd suggest: - Moving ToolBuilder protocol and #parent instvar to UIManager (and recurse on subclasses). - Changing users of ToolBuilder to UIManager. - Changing associated SUnit classes to use UIManager in their names. - Renaming ToolBuilderSpec to UISpec.
How does this sound? I can assist if appropriate.
- One issue I had was trying to recreate the "Search" button that
in the original, took the search field as its model and called #accept on it to kick off a search. In a builder interface, you can't do this, because you only get the specification objects and not the output. In fact, you're not supposed to be able to access the output. I thought about work-arounds, but they all involved propagating the search field text to the model on every keystroke and then having the button call a method on the model that uses that, or making a trampoline in the model that the search field would watch... except that it can't do that. So the search button had to go, because ToolBuilder can't do that easily.
Yes, that is true.
Maybe it can be fixed by detecting that a model is a Spec and then remembering that in an IdentityDictionary and then mapping it to the output. I don't know how feasible that is, but it's seems like it'd work reasonably.
- The other issue was that I couldn't add a help text to the text
field. Why isn't #help an attribute of every specification element type? Who do I bother to get that change made to ToolBuilder? Every object in Squeak needs the ability to explain itself, even if the user turns all of it off (smarter help-showing strategies would probably make it less intrusive).
The best person to bother would be me. And I agree that text fields should have a #help to be used with it. If you post a fix to mantis just assign it to me.
I'll do that, probably with enough of a changeset to start from.
- Trying to layout the search pane *with* the buttons on the top
bar was an exercise in frustration. Passing a fractionally- specified frame rectangle seems like it should be straightforward in semantics, but I couldn't get any layout to happen that corresponded with my intuition, and I still haven't figured out how exactly the builder code deals with those frame specifications. So I moved the search field back to the package- list pane. What am I missing?
Not sure. The layouts should work just as intuitively as you thought. Can you send an example that didn't work the way you expected it?
Okay, here's my explanation: - I've got buildSearchPaneWith: to make the search pane, and buildButtonBarWith: that builds the buttons. - Right now, buildWith: calls the search-pane code and composes it with the other top-level panes to make the whole window. - If I move the buildSearchPaneWith: call into buildButtonBarWith: (before the buttons in my case), then frame: calls don't really work as well. - Usually I get a search pane that is much taller than the buttons, and/or the buttons will not clear past the end of the search pane. - When I had an extra search button, the button wouldn't size to fit its own text and I couldn't figure out what frame: call to get it to fit right. - On the other hand, I was putting the search field and button into their own panel and then putting the panel into the button panel, but even without that extra wrapper, I had the problems with the search field on that row.
PluggableTreeSpec is strange. It took me a while to figure out what kind of protocol I had to make to populate it, or what objects had to respond to the selectors (sometimes it's the item wrapper object, other times the model).
Actually you seem to be misunderstanding something more fundamental about the tree widgets here. From the programmer's point of view there is no "wrapper object" (this is only part of the Morphic implementation but doesn't exist in Tweak for example) and the messages are *always* sent to the model and *always* pass the item along to query for. So, for example, to query the children of an item the tree sends the <getChildren> selector, a one-argument message passing the item alongside it. This avoids the need for any of the wrappers. For example, instead of using a hierarchy of FileDirectoryWrappers you would use something like here:
Aha. The old SMLoader code uses wrappers so I just didn't think I needed to disregard them. FileListPlus actually was the code that I was looking at as an example, but I didn't get this.
MyFileList>>getDirectoryRoots "Answer the roots for the directory tree" ^Array with: FileDirectory root
MyFileList>>getChildrenOf: directory "Answer the children of the directory" ^directory directoryNames collect:[:str| directory directoryNamed: str]
MyFileList>>getLabelOf: directory "Answer the label for the directory" ^directory localName
etc. Again, the idea is to abstract away the current (Morphic) implementation of trees and replace them with a set of pluggable selectors that can be used to query for the relevant properties.
Gotcha. Thanks.
Perhaps there should be a default protocol, a default name for most selectors and a little documentation saying what those are, and that they're override-able in the constructor or mutator methods. (This idea actually reminds me of Rails, and probably rightly so, as they are both builder patterns.) Particularly odd- seeming is how to update the selection programmatically, as happens when the search pane is used - if I call "self changed: #selectedItemWrapperPath", the selector that the package list is watching, it deadlocks trying to redraw until I hit <alt-period>, so I've left that out and the pane doesn't update on its own.
Yes, you'd be interfering with the Morphic implementation in this case. What you want to use is <getSelectedPath> selector from the PluggableTreeSpec, e.g.,
MyModel>>getSelectedDirectoryPath "Answer the path for the currently selected directory" ^selectedDirectory pathParts
and then:
MyModel>>changeDirectoryTo: newDirectory selectedDirectory := newDirectory. self changed: #getSelectedDirectoryPath.
Unfortunately, after making this refactoring, I still have the same problem. I'll elaborate: - I removed any uses of ItemWrappers. - I changed the #selectedItemWrapperPath to #selectedItemPath, returning an Array of the same labels used to build the tree to begin with. - There was a variation in terms of bolding some strings, making them Texts, but removing this variation did not change the outcome (requiring a user-halt).
What I'm reading implicitly is that the path needs to be a sequence of the keys used to drill down to the item. That it is not the sequence of objects themselves stored with those keys. It seems to bear out with the usage, but maybe I'm way off base for reasons of misinterpretation.
When I call "self changed: #selectedItemPath", I get a lock-up which I have to halt which puts me in a stack with PluggableTreeMorph>>update: (on #selectedItemPath) and Array>>at:ifAbsent: at the top.
I've made my latest snapshot available at: http://briantrice.com/Squeak/SMLoader-btr.46.mcz
Uncomment line 3 of SMLoaderPlus>>selectedItem: to see the bug. If someone could look at this with some Morphic knowledge, it'd probably be more effective than my analysis.
-- -Brian http://briantrice.com