Skip to content

New callsites for message browser#19843

Open
AlexisCnockaert wants to merge 2 commits into
pharo-project:Pharo14from
AlexisCnockaert:callsites
Open

New callsites for message browser#19843
AlexisCnockaert wants to merge 2 commits into
pharo-project:Pharo14from
AlexisCnockaert:callsites

Conversation

@AlexisCnockaert

Copy link
Copy Markdown
Collaborator
  • I passed the method browser API to SystemNavigationso the new tabs for the method browser work well

Cnockaert and others added 2 commits June 29, 2026 17:00
- I passed the method browser API to `SystemNavigation`so the new tabs for the method browser work well
@jecisc

jecisc commented Jul 1, 2026

Copy link
Copy Markdown
Member

For this one maybe we should discuss. I already talked with Alexis about that. Maybe it's not the best fix because we have a hidden cyclic dependency.

image

I talked with Guille and he suggested that we update Spec so that it does not use the system navigation to implement the commands but leave a way to configure the commands execution. Then, when the MethodBrowser is loaded, it could tell the code presenter to use it to browser the senders/implementors/references...

@guillep

guillep commented Jul 3, 2026

Copy link
Copy Markdown
Member

The system navigation is not an issue.
Actually, since the presenter is configured with a system navigation, we can change the system navigation by another implementation which is nice.

Now, here the system navigation implements a kind of "callback façade".
The issue here is that in this case, the callback methods were refactored to add a new argument.
So we should think: what are the navigation callbacks that the code presenter should expose, and what is the minimal set of arguments that should be passed/exposed.

I think passing the presenter itself is very flexible.
Then, newtools can define its own callback that receives the code presenter and does its magic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants