search

pharo-project / pharo

Pharo is a dynamic reflective pure object-oriented language supporting live programming inspired by Smalltalk.
http://pharo.org
Other
1.22k stars 356 forks source link

Opening ‘Debug Point Browser’ signals a MessageNotUnderstood #17316

Closed Rinzwind closed 1 month ago

Rinzwind commented 1 month ago

In Pharo 13 build 312, selecting ‘Debug Point Browser’ in the ‘Debug’ menu signals a MessageNotUnderstood (“#copyWith: was sent to nil”).

Rinzwind commented 1 month ago

I checked earlier builds: in build 299 it still works but in build 300 it does not.

Build 300 reference commits:

pharo: 3f903ccbe3e45dc6ee29dd9ec1561fe0d2976cd1
Spec2: c7b733397edc8112bfa4f95a9d9cb80a12a6114f
NewTools: 3f8e5b92780b364eb5caed94ba1ded1a056eca8e
Roassal: 613f15ca24aa4416d86eb7754bb74d78193df1da
DocumentBrowser: 1c2fad2c80a015182431d228d8b3fdf339d0ab31
WelcomeBrowser: c126ba025f217a6db1778f65190b4b44ee42fc02
Microdown: 470ae1ab4cd3be1d93af5d0cf261a3546ffa11f0
tonel: e1dfa3fa6ad7aa69f0393e92929e5c2843919c16

Build 299 reference commits:

pharo: 72a82ad359948072131aa58210281c2c0b7e6138
Spec2: fed16076fed61b1cfa26dacf6692e51f1c017209
NewTools: cdf6246855f2ce8d33cbb497a4f78b792796d820
Roassal: 613f15ca24aa4416d86eb7754bb74d78193df1da
DocumentBrowser: 1c2fad2c80a015182431d228d8b3fdf339d0ab31
WelcomeBrowser: 680e4fa2ca52406bfa742a95f401bf530991f367
Microdown: 470ae1ab4cd3be1d93af5d0cf261a3546ffa11f0
tonel: e1dfa3fa6ad7aa69f0393e92929e5c2843919c16
Rinzwind commented 1 month ago

A comparison of NewTools commits cdf6246855 and 3f8e5b9278 shows that the implementation of #initialize on DebugPointTablePresenter was replaced by an implementation of #initializePresenters, reverting those two changes allows the Debug Point Browser to be opened again. The changes have specifically been made in commit 76e9cf1d5d by @estebanlm.

estebanlm commented 1 month ago

lol, at the end you were right. This is a problem because the author of the debug browser actually extended the table presenter so reverting the initializePresenters move will fix the issue. Problem is that extending spec widgets is not a good idea, in general (I am talking about the atomic widgets, not the presenters which of course, are made to be extended). Anyway, this needs to be refactored eventually, but for now reverting the change will do it.

Rinzwind commented 1 month ago

OK! I guess when making the change you relied on there being a test for opening the browser but there doesn’t seem to be a test like that, so maybe a subclass of SpSmokeTest for DebugPointBrowserPresenter should be added?

Rinzwind commented 1 week ago

I opened a pull request to add a subclass of SpSmokeTest for DebugPointBrowserPresenter: NewTools pull request #900.