-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
GUI rework #399
GUI rework #399
Conversation
Instead of printing out multiple lines for some steps in Layer section, now its only printing one line while other informations can be found in Layer details. This change also provides fix for index out of bounds error when user scrolls through steps in Layer section and there exists at least one step with multi-line commands.
b636f26
to
25194cc
Compare
The Details struct was split into two, LayerDetails and ImageDetails, Each of the three views (Layer, LayerDetails, ImageDetails) takes up a third of the available height of the screen, and they are all now selectable and scrollable.
25194cc
to
2aad87c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! 🚀
This PR is now pending for a while but it would be great if you bring it to master. |
Don't hold your breath, the project hasn't been active in a while. |
Is there any hope, someone performs some kind of CPR? :D |
Sure, feel free to tackle any issues you would like to see solved, just like this one! I'm sure others will appreciate it! |
@mark2185 Do you have a docker image that has these GUI changes? I have this UI issue as well where I can't scroll-down to see the Image and Layer details. |
No, but you can just check out my branch and build it from source. |
I tried building the exe from my laptop (Windows OS) using the "go build" cmd, but when I use the generated .exe to run in a SLES Linux server, it's failing with the error : |
@mark2185 Can you help me by providing the necessary build steps for me to set it up in my SLES server? Thanks. (NOTE : I'm able to run wagoodmans' RPM dive version, maybe is there any way we can build yours' as an rpm one?) |
You need to cross-compile. Try changing the
Then build it in a docker, just run |
@@ -39,5 +41,5 @@ func (l *Layer) String() string { | |||
} | |||
return fmt.Sprintf(LayerFormat, | |||
humanize.Bytes(l.Size), | |||
l.Command) | |||
strings.Split(l.Command, "\n")[0]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider using the solution from #443 instead - replace newlines with the ↵ character but still show the whole command, in case the first line isn't very informative.
(Also, FYI, strings.Split(...)[0]
is equivalent to strings.Cut
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the whole command is shown in the Layer details
pane, this isn't much of a help, although it does help a bit. I'll cherry pick it, thanks!
Thanks for your time on this -- and sorry it took me so long to get to it! The polish looks wonderful, I'll try to get this released 🙌 |
Thanks for getting around to it, I really appreciate it, even if it took a little while! :) |
Looking forward to seeing a new release version available with these changes! |
Do note that #447 introduced a regression by bumping up |
@sschuberth that's correct. |
This should make
dive
much more user friendly, in the sense that all the information is now available, irregardless of the number of layers, or the length of a certain command.The
Layer Details
andImage Details
panes are now selectable with the arrow keys (left and right) and can be scrolled with the arrow keys (up and down). TheImage Details
pane can even be scrolled withPage Up
andPage Down
.I've taken the commit from #395 as well since it fixes a common issue, and I want this PR to be tested by users so why not.
This should close the following issues (and possibly more, I may have missed some):
@wagoodman I know you're busy, but I'd really like your feedback on this. I've had a great deal of fun learning the ins and outs of the project and I'll continue with solving the other issues, but I'd also like to hear your plans for
dive
(if any), and for this all to eventually be merged intomaster
.