The Trunk: Tools-cmm.823.mcz
Locked
 
|
Chris Muller uploaded a new version of Tools to project The Trunk:
http://source.squeak.org/trunk/Tools-cmm.823.mcz ==================== Summary ==================== Name: Tools-cmm.823 Author: cmm Time: 23 June 2018, 6:05:38.664539 pm UUID: 8f7a7bf2-a8e3-497c-a90f-fb2522c558dc Ancestors: Tools-eem.822 Added Debugger class>>#rememberExtent: to allow toggling the debuggers initial extent between either classic RealEstateManager preferences or the extent of the most-recently-closed Debugger. =============== Diff against Tools-eem.822 =============== Item was added: + ----- Method: Debugger class>>rememberExtent: (in category 'opening') ----- + rememberExtent: aBoolean + "Set whether to try to remember the last size of the debugger." + SavedExtent := aBoolean ifTrue: + [ SavedExtent ifNil: + [ "Start at a 33% width, 95% height of the desktop. Use division for its built-in truncation." + Project current world extent // (3@1.05) ] ]! Item was changed: ----- Method: Debugger>>buildFullWith: (in category 'toolbuilder') ----- buildFullWith: builder | windowSpec listSpec textSpec | windowSpec := builder pluggableWindowSpec new model: self; label: 'Debugger'; children: OrderedCollection new. - SavedExtent ifNotNil: - [windowSpec extent: SavedExtent]. listSpec := builder pluggableListSpec new. listSpec model: self; list: #contextStackList; getIndex: #contextStackIndex; setIndex: #toggleContextStackIndex:; menu: #contextStackMenu:shifted:; icon: #messageIconAt:; helpItem: #messageHelpAt:; keyPress: #contextStackKey:from:; frame: (0@0 corner: 1@0.22). windowSpec children add: listSpec. textSpec := self buildCodePaneWith: builder. textSpec frame: (0@0.22corner: 1@0.8). windowSpec children add: textSpec. listSpec := builder pluggableListSpec new. listSpec model: self receiverInspector; list: #fieldList; getIndex: #selectionIndex; setIndex: #toggleIndex:; menu: #fieldListMenu:; keyPress: #inspectorKey:from:; frame: (0@0.8 corner: 0.2@1); help: 'Receiver''s\Instance\Variables' withCRs. windowSpec children add: listSpec. textSpec := builder pluggableTextSpec new. textSpec model: self receiverInspector; getText: #contents; setText: #accept:; help: '<- Select receiver''s field' translated; selection: #contentsSelection; menu: #codePaneMenu:shifted:; frame: (0.2@0.8 corner: 0.5@1). windowSpec children add: textSpec. listSpec := builder pluggableListSpec new. listSpec model: self contextVariablesInspector; list: #fieldList; getIndex: #selectionIndex; setIndex: #toggleIndex:; menu: #fieldListMenu:; keyPress: #inspectorKey:from:; frame: (0.5@0.8 corner: 0.7@1); help: 'Other\Context\Bindings' withCRs. windowSpec children add: listSpec. textSpec := builder pluggableTextSpec new. textSpec model: self contextVariablesInspector; getText: #contents; setText: #accept:; help: '<- Select context''s field' translated; selection: #contentsSelection; menu: #codePaneMenu:shifted:; frame: (0.7@0.8 corner: 1@1). windowSpec children add: textSpec. ^builder build: windowSpec! Item was changed: ----- Method: Debugger>>initialExtent (in category 'initialize') ----- initialExtent "Initial extent for the full debugger. For the notifier's extent see #initialExtentForNotifier." + ^ SavedExtent ifNil: [ 600@700]! - ^ 600@700! Item was changed: ----- Method: Debugger>>windowIsClosing (in category 'initialize') ----- windowIsClosing + "My window is being closed; if debugging save its extent. Clean up. Restart the low space watcher." + interruptedProcess ifNil: [ ^ self ]. + SavedExtent ifNotNil: + [ self dependents + detect: + [ : each | each isWindowForModel: self ] + ifFound: + [ : topWindow | | isDebuggerNotNotifier | + isDebuggerNotNotifier := self dependents anySatisfy: + [ : each | each isTextView ]. + isDebuggerNotNotifier ifTrue: [ SavedExtent := topWindow extent ] ] + ifNone: [ "do nothing" ] ]. - "My window is being closed; if debugging save its extent. - Clean up. Restart the low space watcher." - - interruptedProcess ifNil: [^self]. - (self dependents detect: [:m| m isWindowForModel: self] ifNone: []) ifNotNil: - [:topWindow| | isDebuggerNotNotifier | - isDebuggerNotNotifier := self dependents anySatisfy: [:m| m isTextView]. - isDebuggerNotNotifier ifTrue: - [SavedExtent := topWindow extent]]. interruptedProcess terminate. + interruptedProcess := interruptedController := contextStack := receiverInspector := contextVariablesInspector := nil. + "Restart low space watcher." + Smalltalk installLowSpaceWatcher! - interruptedProcess := nil. - interruptedController := nil. - contextStack := nil. - receiverInspector := nil. - contextVariablesInspector := nil. - Smalltalk installLowSpaceWatcher "restart low space handler" - ! |
Re: The Trunk: Tools-cmm.823.mcz
|
|
Hi Chris, Hi All,
On Sat, Jun 23, 2018 at 4:06 PM, <[hidden email]> wrote: Chris Muller uploaded a new version of Tools to project The Trunk: I don't get this. You've added this but I don't see it sent anywhere. And what of the default 33%@95% height? What if the display is 2560@1440 ? Isn't there some sensible maximum here? I wish instead someone would extend RealEstateManager so that this all but essential feature is available to all system tool windows. Yes, things need resetting or updating on display extent change (for example, scan,ling saved extents by the change in display extent).
_,,,^..^,,,_ best, Eliot |
Re: The Trunk: Tools-cmm.823.mcz
|
|
In reply to this post by commits-2
Hi Chris,
On Sat, Jun 23, 2018 at 4:06 PM, <[hidden email]> wrote: Chris Muller uploaded a new version of Tools to project The Trunk: and the below is broken. If SavedExtent is nil, it won't remember anything. Look, is remembering the last open extent of the debugger a huge improvement or not? I find it a huge improvement. You've just negated that. You consistently protest/alter/reject my contributions. It's getting tedious. Item was changed: _,,,^..^,,,_ best, Eliot |
Re: The Trunk: Tools-cmm.823.mcz
|
|
This is probably just a mistake, or something missing from the commit.
But until that can be sorted out, the Tools-cmm.823 update should probably be reverted and/or moved to the inbox. As a general observation, it is a good idea for all of us to make use of the inbox for changes that may potentially conflict with someone else's work in progress. Merging from inbox to trunk is easy, so it is never a bad idea to use the inbox when we are looking for feedback or review on our changes. Dave On Mon, Jun 25, 2018 at 02:15:44PM -0700, Eliot Miranda wrote: > Hi Chris, > > On Sat, Jun 23, 2018 at 4:06 PM, <[hidden email]> wrote: > > > Chris Muller uploaded a new version of Tools to project The Trunk: > > http://source.squeak.org/trunk/Tools-cmm.823.mcz > > > > ==================== Summary ==================== > > > > Name: Tools-cmm.823 > > Author: cmm > > Time: 23 June 2018, 6:05:38.664539 pm > > UUID: 8f7a7bf2-a8e3-497c-a90f-fb2522c558dc > > Ancestors: Tools-eem.822 > > > > Added Debugger class>>#rememberExtent: to allow toggling the debuggers > > initial extent between either classic RealEstateManager preferences or the > > extent of the most-recently-closed Debugger. > > > > =============== Diff against Tools-eem.822 =============== > > > > Item was added: > > + ----- Method: Debugger class>>rememberExtent: (in category 'opening') > > ----- > > + rememberExtent: aBoolean > > + "Set whether to try to remember the last size of the debugger." > > + SavedExtent := aBoolean ifTrue: > > + [ SavedExtent ifNil: > > + [ "Start at a 33% width, 95% height of the > > desktop. Use division for its built-in truncation." > > + Project current world extent // (3@1.05) ] ]! > > > > Item was changed: > > ----- Method: Debugger>>buildFullWith: (in category 'toolbuilder') ----- > > buildFullWith: builder > > | windowSpec listSpec textSpec | > > windowSpec := builder pluggableWindowSpec new > > model: self; > > label: 'Debugger'; > > children: OrderedCollection new. > > - SavedExtent ifNotNil: > > - [windowSpec extent: SavedExtent]. > > > > listSpec := builder pluggableListSpec new. > > listSpec > > model: self; > > list: #contextStackList; > > getIndex: #contextStackIndex; > > setIndex: #toggleContextStackIndex:; > > menu: #contextStackMenu:shifted:; > > icon: #messageIconAt:; > > helpItem: #messageHelpAt:; > > keyPress: #contextStackKey:from:; > > frame: (0@0 corner: 1@0.22). > > windowSpec children add: listSpec. > > > > > > textSpec := self buildCodePaneWith: builder. > > textSpec frame: (0@0.22corner: 1@0.8). > > windowSpec children add: textSpec. > > > > listSpec := builder pluggableListSpec new. > > listSpec > > model: self receiverInspector; > > list: #fieldList; > > getIndex: #selectionIndex; > > setIndex: #toggleIndex:; > > menu: #fieldListMenu:; > > keyPress: #inspectorKey:from:; > > frame: (0@0.8 corner: 0.2@1); > > help: 'Receiver''s\Instance\Variables' withCRs. > > windowSpec children add: listSpec. > > > > textSpec := builder pluggableTextSpec new. > > textSpec > > model: self receiverInspector; > > getText: #contents; > > setText: #accept:; > > help: '<- Select receiver''s field' translated; > > selection: #contentsSelection; > > menu: #codePaneMenu:shifted:; > > frame: (0.2@0.8 corner: 0.5@1). > > windowSpec children add: textSpec. > > > > listSpec := builder pluggableListSpec new. > > listSpec > > model: self contextVariablesInspector; > > list: #fieldList; > > getIndex: #selectionIndex; > > setIndex: #toggleIndex:; > > menu: #fieldListMenu:; > > keyPress: #inspectorKey:from:; > > frame: (0.5@0.8 corner: 0.7@1); > > help: 'Other\Context\Bindings' withCRs. > > windowSpec children add: listSpec. > > > > textSpec := builder pluggableTextSpec new. > > textSpec > > model: self contextVariablesInspector; > > getText: #contents; > > setText: #accept:; > > help: '<- Select context''s field' translated; > > selection: #contentsSelection; > > menu: #codePaneMenu:shifted:; > > frame: (0.7@0.8 corner: 1@1). > > windowSpec children add: textSpec. > > > > ^builder build: windowSpec! > > > > Item was changed: > > ----- Method: Debugger>>initialExtent (in category 'initialize') ----- > > initialExtent > > "Initial extent for the full debugger. For the notifier's extent > > see #initialExtentForNotifier." > > > > + ^ SavedExtent ifNil: [ 600@700]! > > - ^ 600@700! > > > > > and the below is broken. If SavedExtent is nil, it won't remember > anything. Look, is remembering the last open extent of the debugger a huge > improvement or not? I find it a huge improvement. You've just negated > that. You consistently protest/alter/reject my contributions. It's > getting tedious. > > > > Item was changed: > > ----- Method: Debugger>>windowIsClosing (in category 'initialize') ----- > > windowIsClosing > > + "My window is being closed; if debugging save its extent. Clean > > up. Restart the low space watcher." > > + interruptedProcess ifNil: [ ^ self ]. > > + SavedExtent ifNotNil: > > + [ self dependents > > + detect: > > + [ : each | each isWindowForModel: self ] > > + ifFound: > > + [ : topWindow | | isDebuggerNotNotifier | > > + isDebuggerNotNotifier := self dependents > > anySatisfy: > > + [ : each | each isTextView ]. > > + isDebuggerNotNotifier ifTrue: [ > > SavedExtent := topWindow extent ] ] > > + ifNone: [ "do nothing" ] ]. > > - "My window is being closed; if debugging save its extent. > > - Clean up. Restart the low space watcher." > > - > > - interruptedProcess ifNil: [^self]. > > - (self dependents detect: [:m| m isWindowForModel: self] ifNone: > > []) ifNotNil: > > - [:topWindow| | isDebuggerNotNotifier | > > - isDebuggerNotNotifier := self dependents anySatisfy: [:m| > > m isTextView]. > > - isDebuggerNotNotifier ifTrue: > > - [SavedExtent := topWindow extent]]. > > interruptedProcess terminate. > > + interruptedProcess := interruptedController := contextStack := > > receiverInspector := contextVariablesInspector := nil. > > + "Restart low space watcher." > > + Smalltalk installLowSpaceWatcher! > > - interruptedProcess := nil. > > - interruptedController := nil. > > - contextStack := nil. > > - receiverInspector := nil. > > - contextVariablesInspector := nil. > > - Smalltalk installLowSpaceWatcher "restart low space handler" > > - ! > > > > > > > > > -- > _,,,^..^,,,_ > best, Eliot > |
Re: The Trunk: Tools-cmm.823.mcz
|
|
In reply to this post by Eliot Miranda-2
Hi Eliot,
Your hack is safe and still working as implemented. My change is only to be restore the legacy behavior with: Debugger rememberExtent: false because, as I reported to you days ago, it does not work as designed and, as a result, has made life in the IDE unpleasant. > I don't get this. You've added this but I don't see it sent anywhere. Sent by the user / developer. It's a "undocumented preference" for now until some fix / solution acceptable to everyone can be found. > And what of the default 33%@95% height? What if the display is 2560@1440 ? > Isn't there some sensible maximum here? Does it matter? It's just for the first open, after that it'll remember whatever you sized it to... > I wish instead someone would extend RealEstateManager so that this all but > essential feature is available to all system tool windows. Yes, things need > resetting or updating on display extent change (for example, scan,ling saved > extents by the change in display extent). I took a brief look and considered a "mini-project" to take your idea further, but the development time required combined with the risk of community rejection and low reward payback made such a consideration totally unteneable. > ... snip > > and the below is broken. If SavedExtent is nil, it won't remember anything. Look, is remembering the last open extent of the debugger a huge improvement or not? I find it a huge improvement. You've just negated that. It isn't broken. I apologize if my commit notes aren't clear, I tried to make them so. I also considered writing you a reassuring note, but after considering that you might ignore it, too, I didn't bother. > Look, is remembering the last open extent of the debugger a huge improvement or not? I find it a huge improvement. You've just negated that. You consistently protest/alter/reject my contributions. It's getting tedious. Eliot, the timestamp of the Version in trunk is within 5 minutes of the method timestamps. I Clearly it was not tested, because it doesn't work at all, but it's still there as you put it in. After you ignored my initial inquiry, the only thing I could do was make this bypass, after which I also wrote you a long private email, which you also ignored. I do not wish for you to feel working with me is tedious, but I extended way beyond normal because of who you are. I cannot force you to interact with me, all I can do is try to keep things peaceful by fixing the problem without blowing up your change. Please spend at least that much time looking at my change, reading my commit notes, my prior emails to you before getting upset. Best, Chris |
Re: The Trunk: Tools-cmm.823.mcz
|
|
In reply to this post by David T. Lewis
On Mon, Jun 25, 2018 at 6:40 PM, David T. Lewis <[hidden email]> wrote:
> This is probably just a mistake, or something missing from the commit. > But until that can be sorted out, the Tools-cmm.823 update should > probably be reverted and/or moved to the inbox. Nope, it's fine. There's not much code there, maybe take a look before making such a suggestion? You didn't say anything when I pointed out the original hack doesn't work (and still doesn't), but for whatever reason, you want to revert my code, which does work. :( > As a general observation, it is a good idea for all of us to make > use of the inbox for changes that may potentially conflict with > someone else's work in progress. Merging from inbox to trunk is > easy, so it is never a bad idea to use the inbox when we are looking > for feedback or review on our changes. +1000 |
Re: The Trunk: Tools-cmm.823.mcz
|
|
On Mon, Jun 25, 2018 at 07:04:12PM -0500, Chris Muller wrote:
> On Mon, Jun 25, 2018 at 6:40 PM, David T. Lewis <[hidden email]> wrote: > > This is probably just a mistake, or something missing from the commit. > > But until that can be sorted out, the Tools-cmm.823 update should > > probably be reverted and/or moved to the inbox. > > Nope, it's fine. There's not much code there, maybe take a look > before making such a suggestion? You didn't say anything when I > pointed out the original hack doesn't work (and still doesn't), but > for whatever reason, you want to revert my code, which does work. :( I have not followed this dicussion closely, so I checked back to try to make sense of it. I am still confused. Eliot made a change (Tools-eem.817) to have debugger windows open with a window size matching that of previously opened debugger windows. This may or amy not be a hack but it seems to work as advertised. Chris pointed out a problem, apparently caused by Tools-eem.817, that affects his image: http://lists.squeakfoundation.org/pipermail/squeak-dev/2018-June/199157.html I cannot say the cause of that problem, and I do not know how to reproduce it, but it must be a problem otherwise Chris would not have raised the concern. Chris then introduced rememberExtent: in Tools-cmm.823, which apparently is intended to provide a way to override Eliot's changes. But there are no actual senders of rememberExtent: in the image. Here is what I would say with confidence: A hack to work around somebody else's hack does not belong in the trunk. So I stand by my original statement that it would be good to revert Tools-cmm.823 until the confusion can be sorted out. Having said that, how can I actually reproduce the problem that Chris reported earler in this post? http://lists.squeakfoundation.org/pipermail/squeak-dev/2018-June/199157.html Dave > > > As a general observation, it is a good idea for all of us to make > > use of the inbox for changes that may potentially conflict with > > someone else's work in progress. Merging from inbox to trunk is > > easy, so it is never a bad idea to use the inbox when we are looking > > for feedback or review on our changes. > > +1000 > |
Re: The Trunk: Tools-cmm.823.mcz
|
|
In reply to this post by Chris Muller-3
Hi Chris,
> On Jun 25, 2018, at 4:46 PM, Chris Muller <[hidden email]> wrote: > > Hi Eliot, > > Your hack is safe and still working as implemented. My change is only > to be restore the legacy behavior with: > > Debugger rememberExtent: false > > because, as I reported to you days ago, it does not work as designed > and, as a result, has made life in the IDE unpleasant. But I do not think it does. Because you’re using SavedExtent for both the flag that enables the facility and the value, nilling SavedExtent, or leaving it nil when the class var gets added, the facility is hence off by default. That is a change from my code which enabled the facility by default. A better approach would be to set SavedExtent to a special value, such as -1@-1, to turn off the facility, or use a distinct flag. > >> I don't get this. You've added this but I don't see it sent anywhere. > > Sent by the user / developer. It's a "undocumented preference" for > now until some fix / solution acceptable to everyone can be found. > >> And what of the default 33%@95% height? What if the display is 2560@1440 ? >> Isn't there some sensible maximum here? > > Does it matter? It's just for the first open, after that it'll > remember whatever you sized it to... > >> I wish instead someone would extend RealEstateManager so that this all but >> essential feature is available to all system tool windows. Yes, things need >> resetting or updating on display extent change (for example, scan,ling saved >> extents by the change in display extent). > > I took a brief look and considered a "mini-project" to take your idea > further, but the development time required combined with the risk of > community rejection and low reward payback made such a consideration > totally unteneable. > >> ... snip >> >> and the below is broken. If SavedExtent is nil, it won't remember anything. Look, is remembering the last open extent of the debugger a huge improvement or not? I find it a huge improvement. You've just negated that. > > It isn't broken. I apologize if my commit notes aren't clear, I tried > to make them so. I also considered writing you a reassuring note, but > after considering that you might ignore it, too, I didn't bother. > >> Look, is remembering the last open extent of the debugger a huge improvement or not? I find it a huge improvement. You've just negated that. You consistently protest/alter/reject my contributions. It's getting tedious. > > Eliot, the timestamp of the Version in trunk is within 5 minutes of > the method timestamps. I Clearly it was not tested, because it > doesn't work at all, but it's still there as you put it in. After you > ignored my initial inquiry, the only thing I could do was make this > bypass, after which I also wrote you a long private email, which you > also ignored. > > I do not wish for you to feel working with me is tedious, but I > extended way beyond normal because of who you are. I cannot force you > to interact with me, all I can do is try to keep things peaceful by > fixing the problem without blowing up your change. Please spend at > least that much time looking at my change, reading my commit notes, my > prior emails to you before getting upset. > > Best, > Chris > |
Re: The Trunk: Tools-cmm.823.mcz
|
|
Hi Eliot,
>> because, as I reported to you days ago, it does not work as designed >> and, as a result, has made life in the IDE unpleasant. > > But I do not think it does. Because you’re using SavedExtent for both the flag that enables the facility and the value, nilling SavedExtent, or leaving it nil when the class var gets added, the facility is hence off by default. That is a change from my code which enabled the facility by default. That was fixed by Tools-cmm.825.mcz. > A better approach would be to set SavedExtent to a special value, such as -1@-1, to turn off the facility, or use a distinct flag. Well, even better, IMO, would be a simple shouldSaveExtent ^ SavedExtent notNil for the intention-revealingness of its selector, but I was trying to avoid sparking a method-explosion before we see whether someone wants to step up with support you deserve, our VM developer, and re-do your requirement properly -- integrating with RealEstateManager or whatever. I agree with you it would be a great feature, if it worked, unfortunately, by opening bigger and bigger every time, it actually creates the very problem it's trying to solve, forcing me to have to constantly resize it. - Chris |
Re: The Trunk: Tools-cmm.823.mcz
|
|
Hi Chris,
> On Jun 26, 2018, at 1:42 PM, Chris Muller <[hidden email]> wrote: > > Hi Eliot, > >>> because, as I reported to you days ago, it does not work as designed >>> and, as a result, has made life in the IDE unpleasant. >> >> But I do not think it does. Because you’re using SavedExtent for both the flag that enables the facility and the value, nilling SavedExtent, or leaving it nil when the class var gets added, the facility is hence off by default. That is a change from my code which enabled the facility by default. > > That was fixed by Tools-cmm.825.mcz. Thanks! Sorry, I’m away from my machine today dealing with some urgent family business. I missed your commit. I do apologise. > >> A better approach would be to set SavedExtent to a special value, such as -1@-1, to turn off the facility, or use a distinct flag. > > Well, even better, IMO, would be a simple > > shouldSaveExtent > ^ SavedExtent notNil > > for the intention-revealingness of its selector, but I was trying to > avoid sparking a method-explosion before we see whether someone wants > to step up with support you deserve, our VM developer, and re-do your > requirement properly -- integrating with RealEstateManager or > whatever. I agree with you it would be a great feature, if it worked, > unfortunately, by opening bigger and bigger every time, it actually > creates the very problem it's trying to solve, forcing me to have to > constantly resize it. Is it a window title/border issue that creeps in when smart splitters are enabled? Is it a bug in the builder failing to avoid increasing the window size by the border? Given that there’s essentially one reader and one writer it should be pinpointable. Why not write out its value to the transcript at each point? And write out the window extent after reading SavedWindowExtent on expansion. And sorry this is giving you grief. It should give you joy. Having the debugger open up the same size as last time is so much better than the default. I had a lovely Friday using it. > > - Chris > |
«
Return to Squeak - Dev
|
1 view|%1 views
| Free forum by Nabble | Edit this page |