Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow tagging, pushing, and pulling images #396

Open
jesseduffield opened this issue Oct 25, 2022 · 20 comments
Open

Allow tagging, pushing, and pulling images #396

jesseduffield opened this issue Oct 25, 2022 · 20 comments
Labels
enhancement New feature or request hacktoberfest

Comments

@jesseduffield
Copy link
Owner

Is your feature request related to a problem? Please describe.
It would be good if we could tag, push, and pull images from the images panel.

Describe the solution you'd like

  • In the images panel when you press 't' it should bring up an input prompt for you to enter a tag and then upon pressing enter it should tag the selected image with that tag.
  • When you press 'p' it should bring up a prompt to enter an and tag in the form "image:tag" to pull
  • When you press 'P' it should push the current image, with a confirmation asking if you're sure you want to

Describe alternatives you've considered
The pull feature would benefit from actually trying to search dockerhub, but we can leave that for another PR.

Additional context
the 'p', 'P' thing will be familiar for people who use lazygit

@jesseduffield jesseduffield added enhancement New feature or request hacktoberfest labels Oct 25, 2022
@26tanishabanik
Copy link

@jesseduffield , for pushing the image, how would the authentication be perfomed, or is it like already been taken care of, if yes, where?

@jesseduffield
Copy link
Owner Author

good question. To my knowledge, docker wouldn't ask you to for a password upon trying to push, it would just fail if you're not already logged into the registry. I'm happy for a login flow to be handled in a separate issue

@26tanishabanik
Copy link

good question. To my knowledge, docker wouldn't ask you to for a password upon trying to push, it would just fail if you're not already logged into the registry. I'm happy for a login flow to be handled in a separate issue

Understood. So, which part of this issue can I work on and where should I start for that in the source code?

@jesseduffield
Copy link
Owner Author

All three of the keybindings (tagging, pushing, pulling)

  • the keybindings need to be set in pkg/gui/keybindings.go
  • the handlers for the keybindings need to be created in pkg/gui/images_panel.go

@26tanishabanik
Copy link

26tanishabanik commented Nov 30, 2022

For now, when I press 't' on a selected image, it shows this, with the image name printed on the popUp Panel, but similarly how can I get the input value from the input prompt:
Screenshot 2022-12-01 at 12 56 27 AM

@jesseduffield
Copy link
Owner Author

You can use this as an example:

	return gui.createPromptPanel(gui.Tr.CustomCommandTitle, func(g *gocui.Gui, v *gocui.View) error {
		command := gui.trimmedContent(v)
		return gui.runSubprocess(gui.OSCommand.RunCustomCommand(command))
	})

@26tanishabanik
Copy link

command := gui.trimmedContent(v)

Thanks, that helped, @jesseduffield

@26tanishabanik
Copy link

26tanishabanik commented Dec 1, 2022

Right now, with pull options, after selecting pull out of pull and cancel, it's giving this error. I was trying to pull debain:jessie. I tried docker login also.
Screenshot 2022-12-01 at 1 22 01 PM

@mark2185
Copy link
Contributor

mark2185 commented Dec 1, 2022

@26tanishabanik if you used the string debain:jessie, there's a typo, it's actually debIAn:jessie.

@26tanishabanik
Copy link

@26tanishabanik if you used the string debain:jessie, there's a typo, it's actually debIAn:jessie.

I tried with debain:jessie, @mark2185

@glendsoza
Copy link
Contributor

glendsoza commented Dec 10, 2022

Is someone actively working on this issue, if not I would love to pick this up!

Also I found that ld displays only the first tag in case image is tagged with different tags

....
	for i, image := range images {
		firstTag := ""
		tags := image.RepoTags
		if len(tags) > 0 {
			firstTag = tags[0]
		}
...

is it something that needs to be fixed as users can now tag images

@26tanishabanik
Copy link

Is someone actively working on this issue, if not I would love to pick this up!

Also I found that ld displays only the first tag in case image is tagged with different tags

....
	for i, image := range images {
		firstTag := ""
		tags := image.RepoTags
		if len(tags) > 0 {
			firstTag = tags[0]
		}
...

is it something that needs to be fixed as users can now tag images

I am currently working on it, @glendsoza

@jesseduffield
Copy link
Owner Author

I see that you've taken on a few different issues @26tanishabanik , if you've got other issues in motion it might be worth handing this one off to @glendsoza

@26tanishabanik
Copy link

I see that you've taken on a few different issues @26tanishabanik , if you've got other issues in motion it might be worth handing this one off to @glendsoza

@jesseduffield , I have almost completed the code actually on all those issues, just waiting for your feedback on the question, I asked about the name:tag.
If you want, I can raise a pull request as well

@jesseduffield
Copy link
Owner Author

jesseduffield commented Dec 12, 2022

@26tanishabanik yep go ahead and raise a PR.

In response to your question, you say you used debain:jessie but that itself contains a spelling error. It should be debian:jessie

@26tanishabanik
Copy link

@26tanishabanik yep go ahead and raise a PR.

In response to your question, you say you used debain:jessie but that itself contains a spelling error. It should be debian:jessie

@jesseduffield , I had tried with debian:jessie also, same error, I can show you the screenshots, if you want

@jesseduffield
Copy link
Owner Author

Perhaps raise a PR and then I can pull it down and check it out myself

@jesseduffield
Copy link
Owner Author

@26tanishabanik the problem is that you're passing the image ID to the ImagePull method, and that starts with sha256, which is why it shows up in the error message.

I was originally thinking the user would type in an image and then pull the image that they typed in, but I see the use case for pulling the selected image (and tag). Taking a look at docker desktop, it shows images on a per-tag basis, so we should do the same thing i.e. if an image has two separate tags, there should be two entries in the images panel. You can find the code for that in pkg/commands/image.go RefreshImages().

So with that approach, we can now make it so that when you hit 'p' on an image it brings up a confirmation confirming that you want to pull : again.

We can do the same thing with pushing.

As for tagging, we should actually have two prompts: one for the image name, and one for the image tag. The image name option can be pre-populated with the existing image name.

For pulling, we could have extra functionality that allows you to actually enter the name of the image you want to pull, but I'm happy to have a separate issue for that.

@26tanishabanik
Copy link

@26tanishabanik the problem is that you're passing the image ID to the ImagePull method, and that starts with sha256, which is why it shows up in the error message.

I was originally thinking the user would type in an image and then pull the image that they typed in, but I see the use case for pulling the selected image (and tag). Taking a look at docker desktop, it shows images on a per-tag basis, so we should do the same thing i.e. if an image has two separate tags, there should be two entries in the images panel. You can find the code for that in pkg/commands/image.go RefreshImages().

So with that approach, we can now make it so that when you hit 'p' on an image it brings up a confirmation confirming that you want to pull : again.

We can do the same thing with pushing.

As for tagging, we should actually have two prompts: one for the image name, and one for the image tag. The image name option can be pre-populated with the existing image name.

For pulling, we could have extra functionality that allows you to actually enter the name of the image you want to pull, but I'm happy to have a separate issue for that.

@jesseduffield , if an image is present already, then why to pull it again?
Pushing is fine, I understood
I tagging, why not take the image and tag in the form of image:tag in one prompt?

@jesseduffield
Copy link
Owner Author

You might want to pull again if dealing with an immutable tag like 'latest'

I'm happy with the image:tag approach in one prompt.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request hacktoberfest
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants