Skip to content

Open service in browser - #252

Merged
jesseduffield merged 2 commits into
jesseduffield:masterfrom
jtraub:browser-services
Nov 26, 2021
Merged

Open service in browser#252
jesseduffield merged 2 commits into
jesseduffield:masterfrom
jtraub:browser-services

Conversation

@jtraub

@jtraub jtraub commented Mar 26, 2021

Copy link
Copy Markdown
Contributor

I really liked new features in 0.12 but unfortunately Open in browser doesn't work in services panel.

This PR merely creates the same function for services panel. I took a quick look at pkg/gui/service_panel.go and pkg/gui/containers_panel.go and used copy-paste with slight adoption like I saw in other functions. Sorry if it was bad approach but I have no prior Go experience

@jtraub
jtraub force-pushed the browser-services branch from 7e2e5c2 to 6c27cf3 Compare March 26, 2021 21:15

@mjarkk mjarkk left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good, i would personally make one change to this:

Comment thread pkg/gui/services_panel.go Outdated
Comment on lines +424 to +438
// skip if no any ports
if len(container.Container.Ports) == 0 {
return nil
}
// skip if the first port is not published
port := container.Container.Ports[0]
if port.IP == "" {
return nil
}
ip := port.IP
if ip == "0.0.0.0" {
ip = "localhost"
}
link := fmt.Sprintf("http://%s:%d/", ip, port.PublicPort)
return gui.OSCommand.OpenLink(link)

@mjarkk mjarkk Mar 27, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to be the same code as in container_panel.go > handleContainersOpenInBrowserCommand
Can you abstract this code so we don't have duplicated code?

As you are a bit new to go this is what i would do:

  1. In pkg/commands/container.go add this function func (c *Container) OpenInBrowser() error {
  2. Move the code above to the function created in step 1 (make sure to rename the container var to c)
  3. You can now replace the code above to return container.OpenInBrowser()
  4. Repeat step 3 for container_panel.go > handleContainersOpenInBrowserCommand

@jesseduffield

Copy link
Copy Markdown
Owner

Given this PR was a little stale I've taken the liberty of doing the refactoring :)

@jesseduffield
jesseduffield merged commit 4391878 into jesseduffield:master Nov 26, 2021
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.

3 participants