htop-dev
adding bar_label parameter to htoprc
adding bar_label feature to change the pipe in htop to any other ascii character user wants
this is addressing feature request #647
The PR only has support for normal ascii characters
i can extend this feature to add UTF-8 characters but the CONTRIBUTING.md suggested that newer features shouldn't unnecessarily increase the footprint of the program (this is my first commit btw so i don't know much)
also Please look if the change in the file Settings.c is correct options[1][0] doesn't seem a good way to get the character on right of = sign
While the PR itself looks fine from a code PoV adding new settings is always a bit tricky as future backwards compatibility has to be taken into account. In the mentioned issue, where this new feature is somewhat discussed, the aim was more for something extensible, that may be added upon later if necessary.
As the PR looks now, this is kinda fixed just on this one usecase of just a single character and somewhat neglects the extensibility for e.g. "sub-pixel" highlighting also discussed in the linked issue.
Another aspect is that all settings should be available in the UI. This is currently not the case with this PR.
@fasterit @natoscott : Further thoughts?
Oh thanks for the input, :D I can add sub pixel rendering
maybe an integer called bar_type={1-6} where each number represents the type of bar to use
is this fine or do you guys have any other ideas?
also you said the settings should be available in UI should I add a shortcut key for it?? or when the bar is clicked the pipe cycles through different character-set?
I think for the UI settings it should suffice to have it selectable in the setup screen using F2 similar to how the Hide main function bar setting is handled. Have a look at how number inputs for the settings are done; you can do a new one for the style. This setting can, as you suggested, be an integer in the config file.
Possible future extension: If we really wanted to include a custom option (fully hypothetical), that one could add 18 code points (9 for each bar level) after the initial "custom" designator. But please leave this out for now.
@rustedusted There are two main things to address regarding the whole issue of Bar meter characters:
- The "subpixel" rendering. (Technically it's character cell not pixel, but anyway.) It would require an algorithm rewrite, and while I had some ideas on the algorithm (because I wrote the wonderful #714 feature that's still waiting to be merged), I am still unsure about how useful this "subpixel" rendering of bars can provide comparing to the code complexity that will add. (Also, once implemented, the set of characters to be used would likely be hard-coded, U+2588 - U+258F, as customization on that is rarely useful.)
- The set of characters to be used in a monochrome terminal. It is defined in
BarMeterMode_characters. If we want customizability, it should be customizing that set instead of just one character. But still, I can't see how that customization can be useful.
@rustedusted There are two main things to address regarding the whole issue of Bar meter characters:
- The "subpixel" rendering. (Technically it's character cell not pixel, but anyway.) It would require an algorithm rewrite, and while I had some ideas on the algorithm (because I wrote the wonderful Dynamic scaling & Graph meter coloring (new) #714 feature that's still waiting to be merged), I am still unsure about how useful this "subpixel" rendering of bars can provide comparing to the code complexity that will add. (Also, once implemented, the set of characters to be used would likely be hard-coded, U+2588 - U+258F, as customization on that is rarely useful.)
From a technical PoV the complexity is basically just: Treat the bar meter as having 8 times more space and use the remainder when dividing by 8 as the index into the character table.
One reason for #714 not being merged yet is the heap of complexity it introduces despite what it tries to achieve. That's also why I pushed the "per meter" flags for rendering, as this allows different meter renderings to be used where necessary, thus reducing the complexity in the meter rendering (only has to care that its exact rendering works properly for the data provided).
- The set of characters to be used in a monochrome terminal. It is defined in
BarMeterMode_characters. If we want customizability, it should be customizing that set instead of just one character. But still, I can't see how that customization can be useful.
That set of characters is meant for a different feature and should be handled as such. That other feature is not subject of this PR. Please don't mix up different things.
Hey there sorry for the delay was getting myself acquainted with codebase so i could solve take on more issues
added the sub pixel rendering, however as you might see the bars are distancing themselves between two differently colored bars
should i add sub pixel only for the last bar and keep other bars non subpixel so that it seems consistent instead of disjoint?? or should i make the background of the ending characters of bars to be the foreground color of next bar?
if you have any other idea please let me know i feel like the next commits will be much faster now that i know significantly more about the code ig :smile:
i have added everything in the following commits that was suggested
if there is anything more please let me know
Oh hey there you're right about squashing I didn't know a lot about git just learned few things will make a new pull request to do this
I'm really very sorry for the inconvenience
@rustedusted What are you doing? Try git rebase with -i option and it can help you with many things. No need to make another pull request.
in fact i didn't know i had to create a new branch for that matter this was my first commit tbh
is there a way to recover this?!
this is the entire history of commits all the commits should be squashed ig
@rustedusted I don't recommend breaking existing discussions so I would suggest you to keep this branch main for now unless you really want the PR to be closed.
You can set up a new branch upstream-main (using git branch command) and have upstream-main follow the main of this repository, and you won't get conflicts.
hey there just an update i'm working on this issue couldn't work on this yesterday will definitely try putting a commit tomorrow
i've made a branch with appropriate changes named new-stream
do i make a new pull request?? @Explorer09
hey also wanted to try out solving other issues can i take a new issue as i feel like this one is about to be solved ig idk tbh:|
do i make a new pull request??
No, just force-push your updated PR on the branch you used to make this one. No need for a new PR, as this splits the discussion unnecessarily.
can you tell me how to do it i asked on irc and stuff but couldn't get much answers
the branch that i made pr with is main and the branch that contains updated and correct changes is called new-stream
how do i make the origin/main of my repo follow new-stream in my local
i tried doing git push -u origin main while checked out in new-stream
most people are just telling me to make new pull request
how do i make the origin/main of my repo follow new-stream in my local
git checkout new-stream # Checkout the branch you want to modify
git branch --set-upstream-to=origin/main # Update the upstream marker
git push --force-with-lease # Send the (new) commits to the server after rewriting history
hey there finally done that thank you so much for the gb --set-upstream-to command i didn't even think it was possible
i have done the appropriate changes tell me any more changes if there are any
Rearranged some lines to better group the changes. Also fixed some code style issues.
what are the issues here?!! it says 15 failing did i do something wrong?!
Thanks for helping me out @BenBE also i had to ask if there's any way to solve all the checks
Thanks for helping me out @BenBE also i had to ask if there's any way to solve all the checks
Have you tried compiling this locally?
From the build log:
depbase=`echo DisplayOptionsPanel.o | sed 's|[^/]*$|.deps/&|;s|\.o$||'`;\
gcc -DHAVE_CONFIG_H -I. -DNDEBUG -DGCC_PRINTF -DGCC_SCANF -D_FORTIFY_SOURCE=2 -std=c99 -pedantic -D_DEFAULT_SOURCE -D_XOPEN_SOURCE=600 -Wall -Wcast-align -Wcast-qual -Wextra -Wfloat-equal -Wformat=2 -Winit-self -Wmissing-format-attribute -Wmissing-noreturn -Wmissing-prototypes -Wpointer-arith -Wshadow -Wstrict-prototypes -Wundef -Wunused -Wwrite-strings -Wnull-dereference -Werror -D_XOPEN_SOURCE_EXTENDED -DSYSCONFDIR="\"/usr/local/etc\"" -I"./linux" -g -O2 -I/usr/include/libunwind -MT DisplayOptionsPanel.o -MD -MP -MF $depbase.Tpo -c -o DisplayOptionsPanel.o DisplayOptionsPanel.c &&\
mv -f $depbase.Tpo $depbase.Po
DisplayOptionsPanel.c: In function ‘DisplayOptionsPanel_new’:
DisplayOptionsPanel.c:159:[69](https://github.com/htop-dev/htop/actions/runs/11323101825/job/31485077594?pr=1546#step:7:70): error: passing argument 2 of ‘NumberItem_newByRef’ from incompatible pointer type [-Werror=incompatible-pointer-types]
159 | Panel_add(super, (Object*) NumberItem_newByRef("Bar Type (0-7)", &(settings->barType), 0, 0, 7));
| ^~~~~~~~~~~~~~~~~~~~
| |
| uint8_t * {aka unsigned char *}
In file included from DisplayOptionsPanel.c:20:
OptionItem.h:[73](https://github.com/htop-dev/htop/actions/runs/11323101825/job/31485077594?pr=1546#step:7:74):56: note: expected ‘int *’ but argument is of type ‘uint8_t *’ {aka ‘unsigned char *’}
73 | NumberItem* NumberItem_newByRef(const char* text, int* ref, int scale, int min, int max);
| ~~~~~^~~
cc1: all warnings being treated as errors
yeah i did compile it on my machine i did that using
./autogen.sh && ./configure --enable-debug --enable-unicode && make && ./htop
the above command
it worked at least when i've tried it on my computer
i'm using ubuntu 22.04 right now
oh hey there very sorry i did not notice the warning i thought we were compiling with Werror and Wall flag
done i don't think there should be any warnings now also i saw the snippet you sent
cc1: all warnings being treated as errors
this was a message in the snippet you sent i don't get such messages in fact it just showed warnings and ran without any issue