Support bulk commands in each of the panels #107

jesseduffield · 2019-07-06T01:48:12Z

This PR:

Rolls out the custom commands functionality to each of the side panels (except the top side panel)
Adds some configurable bulk commands like docker-compose up -d and docker container prune for the side panels.

golangcibot · 2019-07-06T01:49:49Z

pkg/gui/images_panel.go

@@ -259,3 +259,18 @@ func (gui *Gui) handlePruneImages(g *gocui.Gui, v *gocui.View) error {
 		})
 	}, nil)
 }
+
+func (gui *Gui) handleImagesCustomCommand(g *gocui.Gui, v *gocui.View) error {


U1000: func (*Gui).handleImagesCustomCommand is unused (from unused)

golangcibot · 2019-07-06T01:49:49Z

pkg/gui/volumes_panel.go

@@ -250,3 +250,18 @@ func (gui *Gui) handlePruneVolumes(g *gocui.Gui, v *gocui.View) error {
 		})
 	}, nil)
 }
+
+func (gui *Gui) handleVolumesCustomCommand(g *gocui.Gui, v *gocui.View) error {


U1000: func (*Gui).handleVolumesCustomCommand is unused (from unused)

codecov-io · 2019-07-06T01:49:52Z

Codecov Report

Merging #107 into master will increase coverage by 2.25%.
The diff coverage is 90.24%.

@@            Coverage Diff             @@
##           master     #107      +/-   ##
==========================================
+ Coverage   23.71%   25.97%   +2.25%     
==========================================
  Files          13       13              
  Lines        1088     1132      +44     
==========================================
+ Hits          258      294      +36     
- Misses        817      825       +8     
  Partials       13       13

Impacted Files	Coverage Δ
pkg/commands/docker.go	`0% <ø> (ø)`	⬆️
pkg/commands/container.go	`0% <0%> (ø)`	⬆️
pkg/config/app_config.go	`82.27% <100%> (+5.22%)`	⬆️
pkg/commands/container_stats.go	`0% <0%> (ø)`	⬆️

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 64863ad...bd3ce66. Read the comment docs.

mcintyre94 · 2019-07-09T10:31:52Z

Nice one! I really like the use of a separate key to open bulk shortcuts, looks like it'll work much better than combining with the x shortcuts since it's being applied to all panels. Also good shout to expand to all side panels :)

Does this look right?

Looks really good to me - definitely got the ones I'd be looking for there.

Should we now remove the hardcoded shift+d keybinding which was for pruning all containers, given that you'll now be able to do it through the menu?

Probably, using the bulk menu seems clearer IMO.

Did we have plans to remove all CLI usage at some point? If so, this is adding tech debt. Although it's worth considering whether configurability is more important than whatever we hoped to get out of moving away from the CLI commands

I wasn't aware of that, do you know where it was discussed? Agree this adds tech debt if that was a goal, not sure how else we'd do it though. Is there a go lib for docker-compose like there is for docker (assuming that's what docker CLI would be replaced with)?

Should we have a waiting status (the thing you see at the bottom left corner while waiting) for each of these commands? Currently it just says 'running bulk command...'

Seems pretty minor, but maybe we could customise it per panel? 'running bulk container command' / 'running bulk volume command' etc.?

A few thoughts from running this locally at work:

Overall it works really well! Pull, build, up are all great, and it's a much much easier workflow than what we previously used.
in the bottom status bar with x: menu it'd be good to add b: bulk commands or similar :)
Also what would you think about adding a up command without the -d, that would drop you into the console while it runs, like build does? That'd be useful for debugging, when I had an issue with up -d I had to drop out into docker-compose to run it without the flag and see what was going on.
Maybe docker-compose build --parallel? It looks like unlike pull build isn't parallel by default.

golangcibot · 2019-07-12T12:19:45Z

pkg/gui/containers_panel.go

+		return gui.WithWaitingStatus(gui.Tr.StoppingStatus, func() error {
+
+			for _, container := range gui.DockerCommand.Containers {
+				container.Stop()


Error return value of container.Stop is not checked (from errcheck)

golangcibot · 2019-07-12T12:19:46Z

pkg/gui/containers_panel.go

+		return gui.WithWaitingStatus(gui.Tr.RemovingStatus, func() error {
+
+			for _, container := range gui.DockerCommand.Containers {
+				container.Remove(types.ContainerRemoveOptions{Force: true})


Error return value of container.Remove is not checked (from errcheck)

jesseduffield · 2019-07-12T12:22:30Z

Alright I've made some changes. Now we're not using the docker CLI directly for anything, we're using the docker SDK.

I've added a new key on our custom command struct called InternalFunction which is the name of a whitelisted internal function e.g. 'pruneContainers'. This is so that we can have custom commands that point to internal functions sitting alongside custom commands that have their own CLI commands.

I am a little concerned about the fact that I'm introducing a concept that's not only not type safe, but is spread across a few files. The mapping of whitelisted functions is in custom_commands.go, but referenced in app_config.go, and the actual functions themselves are in the corresponding _panels file. I can't move the mapping into app_config.go because that file has no concept of the gui struct.

An alternative would be to have some stock standard custom commands that are not configurable, and those are for the internal functions. Then the remaining custom commands are configurable. I'm leaning towards this approach, but it's going to be a bit messier than what we have now.

Any thoughts @mcintyre94 ?

mcintyre94 · 2019-07-14T17:03:35Z

Unfortunately I'm not very well positioned to do Go code review! I can see what you're refering to though, it looks like it could get a bit tricky to maintain. I wonder if it'd make sense to add a struct for the valid keys of that map, which could (I think?) be available to app_config.go as well as custom_commands.go? Might just be adding another layer of indirection, but it'd at least be really obvious if something was added to custom_commands.go that wasn't in that keys map, and at least saves any typos referencing them in app_config.go.

Or is that what you meant by having some stock commands that aren't configurable? I think I would lean toward something a bit safer but not sure what the best way of achieving that would be.

jesseduffield · 2019-07-21T21:06:29Z

Yeah it seems like the right way to do this is to have some bulk commands that are there by default, which aren't configurable and just reference internal functions, and then allow the user to add some extra bulk commands on top which can't reference internal functions. Then we don't need to worry about type safety or keeping things synced up or anything. I'll try to make that happen this week.

jesseduffield · 2019-07-27T05:16:31Z

does anybody want to have a look at this PR before I merge? @dawidd6 @mjarkk? @glvr182

dawidd6 · 2019-07-27T08:15:13Z

It would be also a good idea to update README if you bumped API version.

mjarkk

LGTM

glvr182

LGTM, seems like I need to update the custom keybindings a lot :D

update api version in readme

jesseduffield changed the title ~~support bulk commands in each of the panels~~ WIP: support bulk commands in each of the panels Jul 6, 2019

jesseduffield mentioned this pull request Jul 6, 2019

Add some docker-compose config-wide shortcuts #78

Closed

golangcibot reviewed Jul 6, 2019

View reviewed changes

jesseduffield force-pushed the bulk-commands branch from 772d0f6 to b6a97ce Compare July 7, 2019 01:54

jesseduffield force-pushed the bulk-commands branch 3 times, most recently from 3c0ca75 to 667911c Compare July 12, 2019 12:18

golangcibot reviewed Jul 12, 2019

View reviewed changes

jesseduffield force-pushed the bulk-commands branch 2 times, most recently from 14a5cd9 to 82aba25 Compare July 27, 2019 05:12

jesseduffield changed the title ~~WIP: support bulk commands in each of the panels~~ Support bulk commands in each of the panels Jul 27, 2019

mjarkk approved these changes Jul 27, 2019

View reviewed changes

glvr182 approved these changes Jul 27, 2019

View reviewed changes

jesseduffield added 2 commits July 29, 2019 18:59

support bulk commands in each of the panels

1f9f826

bump api version so that we can e.g. prune containers via the sdk

bd3ce66

update api version in readme

jesseduffield force-pushed the bulk-commands branch from 2ccb394 to bd3ce66 Compare July 29, 2019 08:59

jesseduffield merged commit 4dc8f76 into master Jul 29, 2019

delete-merged-branch bot deleted the bulk-commands branch July 29, 2019 09:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support bulk commands in each of the panels #107

Support bulk commands in each of the panels #107

jesseduffield commented Jul 6, 2019 •

edited

Loading

golangcibot Jul 6, 2019

golangcibot Jul 6, 2019

codecov-io commented Jul 6, 2019 •

edited

Loading

mcintyre94 commented Jul 9, 2019 •

edited

Loading

golangcibot Jul 12, 2019

golangcibot Jul 12, 2019

jesseduffield commented Jul 12, 2019

mcintyre94 commented Jul 14, 2019

jesseduffield commented Jul 21, 2019

jesseduffield commented Jul 27, 2019

dawidd6 commented Jul 27, 2019

mjarkk left a comment

glvr182 left a comment

Support bulk commands in each of the panels #107

Support bulk commands in each of the panels #107

Conversation

jesseduffield commented Jul 6, 2019 • edited Loading

golangcibot Jul 6, 2019

Choose a reason for hiding this comment

golangcibot Jul 6, 2019

Choose a reason for hiding this comment

codecov-io commented Jul 6, 2019 • edited Loading

Codecov Report

mcintyre94 commented Jul 9, 2019 • edited Loading

golangcibot Jul 12, 2019

Choose a reason for hiding this comment

golangcibot Jul 12, 2019

Choose a reason for hiding this comment

jesseduffield commented Jul 12, 2019

mcintyre94 commented Jul 14, 2019

jesseduffield commented Jul 21, 2019

jesseduffield commented Jul 27, 2019

dawidd6 commented Jul 27, 2019

mjarkk left a comment

Choose a reason for hiding this comment

glvr182 left a comment

Choose a reason for hiding this comment

jesseduffield commented Jul 6, 2019 •

edited

Loading

codecov-io commented Jul 6, 2019 •

edited

Loading

mcintyre94 commented Jul 9, 2019 •

edited

Loading