wagoodman / dive

A tool for exploring each layer in a docker image
MIT License
46.65k stars 1.77k forks source link

Fix scrolling after gocui update #473

Open mark2185 opened 1 year ago

mark2185 commented 1 year ago

This fixes the scrolling caused by a regression in #447 .

It still has some issues regarding highlighting rows, but I'm not overtly interested in diving into gocui given that it's not actively maintained. At least it enables the user to scroll again.

vinkmr commented 1 year ago

Built using go1.21.0 and tested on Ubuntu 20.04. Unfortunately the scrolling bug is still there. :(

mark2185 commented 1 year ago

Ah, I see I've only fixed it for the Image Details pane, thanks.

mark2185 commented 1 year ago

I took the liberty of rebasing onto #478 , give it another spin @vinkmr .

vinkmr commented 1 year ago

Yup. Scroll seems to work for me now.

marcusramberg commented 1 year ago

Would be great to see a new release with this fix, as it severely limits the usability of dive for nix-based images :)

glensc commented 10 months ago

To run this branch

$ gh repo clone https://github.com/wagoodman/dive         
Cloning into 'dive'...
remote: Enumerating objects: 5334, done.
remote: Counting objects: 100% (1258/1258), done.
remote: Compressing objects: 100% (315/315), done.
remote: Total 5334 (delta 1007), reused 1110 (delta 932), pack-reused 4076
Receiving objects: 100% (5334/5334), 5.05 MiB | 5.26 MiB/s, done.
Resolving deltas: 100% (3500/3500), done.

$ cd dive 

$ gh co 473           
From https://github.com/wagoodman/dive
 * [new ref]         refs/pull/473/head -> fix/scrolling-contents
Switched to branch 'fix/scrolling-contents'

$ go run main.go php:7.2-fpm
MauriceArikoglu commented 10 months ago

@wagoodman @mark2185 whats missing for this PR to get merged? Want to offer help 🤝

mehmetumit commented 9 months ago

Seems like these lines need to be fixed in order to pass ci check

runtime/ui/view/layer.go:298:14: Error return value of `v.vm.Update` is not checked (errcheck)
                v.vm.Update(v.constrainedRealEstate)
                           ^
runtime/ui/viewmodel/layer_set_state.go:42:26: func `(*LayerSetState).height` is unused (unused)
func (vm *LayerSetState) height() int {
                         ^
runtime/ui/view/layer.go:153:17: func `(*Layer).height` is unused (unused)
func (v *Layer) height() uint {
                ^
black-snow commented 7 months ago

Please release this - super annoying bug.

ruffsl commented 7 months ago

I just had to rebase this PR on top of the current head ( 925cdd86482edec42185794620a1e616b79bbee5 ) of main branch to work around something similar to this:

Thanks for the patch!

MauriceArikoglu commented 7 months ago

Any update? @wagoodman @mehmetumit @mark2185 can you fix the mentioned ci issues?

LaTrissTitude commented 7 months ago

Would be great for this fix to reach the next release

black-snow commented 7 months ago

Sadly, the logs have expired already.

pov1ba commented 6 months ago

You can still see what was the issue here. https://github.com/wagoodman/dive/actions/runs/6391362920?pr=473

I made another pull request based on this one here, #520, that should hopefully pass all the checks. I just need someone to review and approve it now.

Will be nice if we can finally have this fix in new versions.

uuf6429 commented 4 months ago

This is arguably one of the main features of this tool - and this bug's been open since over a year apparently, with devs asking for its release and offering their time and help. What's the hold up then?

PS: On a related point, the "Layer Details" panel takes up 1/3 of the vertical screen area when in fact it seems to require at most 9 lines (which means, less space the "Layers" panel, making this issue even worse).

mark2185 commented 4 months ago

What's the hold up then?

Can't speak for wagoodman since I'm not him, but just a reminder that this is a free, non-paid project, that nobody is forced to develop and/or maintain, there are no deadlines or milestones it has to reach, so we can't really blame anyone.

If you wish to improve upon it, feel free to fork it. But do remember that we're all human, and life sometimes gets in the way.

featheredtoast commented 2 months ago

@mark2185 thanks so much for this patch - The branch might be worth a rebase so the fix still "just works" as is.

I had to rebase your fix on top of the latest version (OCI images on mac weren't loading) but otherwise it worked great.