Skip to content

Show Project Directory in Status Panel - #88

Merged
jesseduffield merged 6 commits into
jesseduffield:masterfrom
pvande:feature/display-project-directory
Jul 5, 2019
Merged

Show Project Directory in Status Panel#88
jesseduffield merged 6 commits into
jesseduffield:masterfrom
pvande:feature/display-project-directory

Conversation

@pvande

@pvande pvande commented Jul 3, 2019

Copy link
Copy Markdown
Contributor

As per this discussion, this puts the current working directory name into the Status Panel area. As related changes, this PR also moves the "lazydocker" project name down to the information area, and renames the Status Panel to the "Project Panel" (which seemed a natural consequence).

Comment thread pkg/gui/project_panel.go
}

gui.handleStatusSelect(gui.g, v)
gui.handleProjectSelect(gui.g, v)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Error return value of gui.handleProjectSelect is not checked (from errcheck)

Comment thread pkg/gui/project_panel.go
}

gui.handleStatusSelect(gui.g, v)
gui.handleProjectSelect(gui.g, v)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Error return value of gui.handleProjectSelect is not checked (from errcheck)

@dawidd6

dawidd6 commented Jul 3, 2019

Copy link
Copy Markdown
Collaborator

Please also adjust this line:

mConfig, err := config.NewAppConfig("lazydocker", "", "", "", "", true, nil)

@codecov-io

Copy link
Copy Markdown

Codecov Report

Merging #88 into master will increase coverage by 0.07%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #88      +/-   ##
==========================================
+ Coverage   23.61%   23.68%   +0.07%     
==========================================
  Files          13       13              
  Lines        1080     1081       +1     
==========================================
+ Hits          255      256       +1     
  Misses        813      813              
  Partials       12       12
Impacted Files Coverage Δ
pkg/config/app_config.go 77.39% <100%> (+0.19%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 572ff63...f9ba4b8. Read the comment docs.

@codecov-io

codecov-io commented Jul 3, 2019

Copy link
Copy Markdown

Codecov Report

Merging #88 into master will increase coverage by 0.07%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #88      +/-   ##
==========================================
+ Coverage   23.61%   23.68%   +0.07%     
==========================================
  Files          13       13              
  Lines        1080     1081       +1     
==========================================
+ Hits          255      256       +1     
  Misses        813      813              
  Partials       12       12
Impacted Files Coverage Δ
pkg/config/app_config.go 77.39% <100%> (+0.19%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 572ff63...b3422c8. Read the comment docs.

@pvande

pvande commented Jul 3, 2019

Copy link
Copy Markdown
Contributor Author

This would arguably be more correct if it preferred to use the "com.docker.compose.project" label...

docker inspect --format='{{json .Config.Labels }}' $OBJECT_ID
{
"com.docker.compose.config-hash":"fa59a5b47221e5a6a9b4d4182709755a0079ae2b4b634f59c44108a6dd57d081",
"com.docker.compose.container-number":"1",
"com.docker.compose.oneoff":"False",
"com.docker.compose.project":"project-name",
"com.docker.compose.service":"anonymous-service",
"com.docker.compose.version":"1.23.2"
}

I don't know how often anyone actually customizes the project name, though...

@jesseduffield

Copy link
Copy Markdown
Owner

@pvande coincidentally, my work does haha. I'm not sure what the best approach is here. Is there a way to check if the project name hasn't been custom-set, and if so just revert to using the initial approach?

@pvande

pvande commented Jul 3, 2019

Copy link
Copy Markdown
Contributor Author

@jesseduffield Awesome! As near as I can tell, the only way to get the project name back out of docker-compose is through the container labels. That's theoretically easy enough, though it introduces a strange cross-panel dependency (the Status/Project Panel now needs the list of Services).

I'll see if I can get a PoC together quickly.

Comment thread pkg/gui/project_panel.go
func (gui *Gui) getProjectContexts() []string {
if gui.DockerCommand.InDockerComposeProject {
return []string{"logs", "credits", "config"}
return []string{"logs", "config", "credits"}

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I'm cool with this

@jesseduffield

Copy link
Copy Markdown
Owner

LGTM. One last thing though, given that we will likely have a bit of free horizontal space in the project panel, I wonder if we should add in brackets the docker-compose command from the config. That way, if people start lazydocker like lazydocker -f docker-compose.test.yml we can say

your-project-name (docker-compose -f docker-compose.test.yml)

That way they have even more context about what they're seeing. What are your thoughts on that?

@pvande

pvande commented Jul 4, 2019

Copy link
Copy Markdown
Contributor Author

@jesseduffield I'm not opposed to surfacing the docker-compose command, but I'm not myself convinced that after the project name in the project panel is the best place for it.

While we will have some space after the project name, even the simple example you gave is 62 characters long. At my desk, on my 4K monitor, that would probably fit just fine, but when I'm out on my laptop, my entire terminal width is commonly between 110 and 230 characters. That doesn't really leave enough space to show any meaningful amount of that command.

It might be better to put that information on a second line in the project panel (space is still an issue, but less so), or in a tab under the project panel (perhaps fleshing the config tab out with more than just the literal config text).

@jesseduffield

Copy link
Copy Markdown
Owner

I agree. We can have another look at that after this is merged. As for this PR, it LGTM. Anything else you needed to do before we merge it?

@pvande

pvande commented Jul 5, 2019

Copy link
Copy Markdown
Contributor Author

Anything further I might contribute fits just as easily in a new PR. Merge away! 🙂

@jesseduffield
jesseduffield merged commit 9207dc6 into jesseduffield:master Jul 5, 2019
@jesseduffield

Copy link
Copy Markdown
Owner

thanks again :)

@pvande
pvande deleted the feature/display-project-directory branch July 5, 2019 00:42
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.

5 participants