Introduce screen tabs #780
Conversation
|
This pull request introduces 1 alert when merging e781d95 into a516e08 - view on LGTM.com new alerts:
|
|
This pull request introduces 1 alert when merging 11f453a into a516e08 - view on LGTM.com new alerts:
|
|
Great work as usual, @natoscott :) Bug 1) missing mouse handling Bug 2) htoprc & settings related (I think!) |
natoscott
force-pushed
the
tabs
branch
4 times, most recently
from
074e4a4 to
4717923
Compare
Thanks @smalinux - these issues should be resolved now with that last git push. |
natoscott
force-pushed
the
tabs
branch
2 times, most recently
from
96ebf10 to
f15c2b9
Compare
Settings.c
Outdated
| String_freeArray(ids); | ||
| } | ||
|
|
||
| ScreenSettings* Settings_newScreen(Settings* this, const char* name, const char* line) { | ||
| ScreenSettings* ss = xCalloc(sizeof(ScreenSettings), 1); |
There was a problem hiding this comment.
I think this pointer didn't free properly. I will try to debug this later,
ASan output:
==195406==ERROR: LeakSanitizer: detected memory leaks
Direct leak of 40 byte(s) in 1 object(s) allocated from:
#0 0x7f4adc282dc6 in calloc (/lib/x86_64-linux-gnu/libasan.so.5+0x10ddc6)
#1 0x559095bdcb64 in xCalloc /home/smalinux/htop/XUtils.c:55
#2 0x559095bd3546 in Settings_newScreen /home/smalinux/htop/Settings.c:266
#3 0x559095bd38dd in Settings_defaultScreens /home/smalinux/htop/Settings.c:289
#4 0x559095bd3b2b in Settings_defaultScreens /home/smalinux/htop/Settings.c:285
#5 0x559095bd3b2b in Settings_read /home/smalinux/htop/Settings.c:328
#6 0x559095bd7fbc in Settings_new /home/smalinux/htop/Settings.c:732
#7 0x559095b9f988 in CommandLine_run /home/smalinux/htop/CommandLine.c:303
#8 0x7f4adbdc30b2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x270b2)
SUMMARY: AddressSanitizer: 40 byte(s) leaked in 1 allocation(s).
There was a problem hiding this comment.
xCalloc like calloc takes the number of elements first and the size of each element as second. Mostly a side-note, but having them swapped may lead to unnecessary trouble down the line.
That's most likely not the bug you're chasing, but you're advised to fix it while at it. ;-)
There was a problem hiding this comment.
@smalinux thanks, I have a fix now - will push shortly.
|
Previous few commits from @smalinux address the final remaining feedback items (incl. defaulting to tabs disabled on upgrade). If there's other areas for improvement let us know please otherwise we'll go ahead and merge soon - thanks! |
This is a forward port (by nathans) of Hisham's original code.
If the new htop is configured with htoprc having no tabs (eg on upgrade) then the interface will not automatically introduce/enable them. However, for a fresh install of htop, enabling them automatically Signed-off-by: Sohaib Mohamed <sohaib.amhmd@gmail.com>
Signed-off-by: Sohaib Mohamed <sohaib.amhmd@gmail.com>
Signed-off-by: Sohaib Mohamed <sohaib.amhmd@gmail.com>
Also tidy up the calloc call parameters in the initial allocation of this pointer, thanks to @BenBE for noticing.
|
@natoscott Yay, I was thinking about that branch the other day and just learned about this work! Thank you so much! ❤️ (I just learned about this by clicking through from your post on the GSoC mailing list — clearly my Github notifications are a lost cause!) |
| @@ -714,4 +756,6 @@ void Action_setBindings(Htop_Action* keys) { | |||
| keys[KEY_F(10)] = actionQuit; | |||
| keys[KEY_F(18)] = actionExpandCollapseOrSortColumn; | |||
| keys[KEY_RECLICK] = actionExpandOrCollapse; | |||
| keys[KEY_SHIFT_TAB] = actionPrevScreen; | |||
There was a problem hiding this comment.
This key is Ctrl+F9
See [Function Keys]
There was a problem hiding this comment.
May be on AIX. Which is the documentation you are referring to. On Linux it is Shift-TAB and that works as you can easily test yourself.
Signed-off-by: Sohaib Mohamed <sohaib.amhmd@gmail.com>
This is a forward port (by nathans) of Hisham's original code.