Remove BrowserUtils.OpenDefaultBrowser and use ISessionManager#2261
Conversation
Given that the ISessionManager has the ability to determine if a web browser is available (via `IsWebBrowserAvailable`), it's only natural that this is the component that should also be responsible for actually launching the browser. We remove the `BrowserUtils` class and transpose its only method (`OpenDefaultBrowser`) to the session manager component. This means we can keep the Linux-specific behaviour in a scoped component. Signed-off-by: Matthew John Cheetham <mjcheetham@outlook.com>
mjcheetham
force-pushed
the
browser-on-sessionmanager
branch
from
3c6ada5 to
a344fe9
Compare
dscho
left a comment
dscho
left a comment
There was a problem hiding this comment.
Wow, this requires a lot of changes! But the intention makes sense to me, and I see that there is no way to make this change more minimal.
Yeah, the We could update the call-sites piece meal, but each commit would leave things uncompilable? Thoughts? |
No, I'd prefer it to be compilable. Those changes that update the call sites are somewhat repetitive, but easy to verify, and the fact that the result compiles cleanly proves that no call site was missed. So it's all good to go from my side. |
2 participants
Given that the ISessionManager has the ability to determine if a web browser is available (via
IsWebBrowserAvailable), it's only natural that this is the component that should also be responsible for actually launching the browser.We remove the
BrowserUtilsclass and transpose its only method (OpenDefaultBrowser) to the session manager component. This means we can keep the Linux-specific behaviour in a scoped component.