feat(colors): Adding settings to change color when speed threshold is… #4
Conversation
|
Hi @Brams-s, thank you for the PR for the feature, and the refactoring too. This looks very useful. I will have a look at this tonight (in UTC+7). |
network_speed.sh
Outdated
| fi | ||
| } | ||
| # Retrieve tmux options for speed threshold and high speed color, set default if not found | ||
| threshold_speed="$(get_tmux_option "@network_speed_threshold" "1.0")" |
There was a problem hiding this comment.
I think the default value for this should be 0. If we default to 1.0, people may see red color for upload or download speed when they don't expect it to be. Let's not apply this threshold by default. If someone want this they can explicitly set the threshold. What do you think?
There was a problem hiding this comment.
Completely agree, hadn't really considered that yet but makes a lot of sense to me.
helpers.sh
Outdated
| local speed_unit=$(echo $speed | awk '{print $2}') | ||
|
|
||
| # If speed unit is MB/s and speed value is greater than the threshold, use high color | ||
| if [[ $speed_unit == "MB/s" ]] && (($(echo "$speed_value > $threshold" | bc -l))); then |
There was a problem hiding this comment.
Can we be flexible with the speed_unit? I mean with this we can only use this feature with MB, not for KB or maybe GB in the future. An approach I can think of is to use the byte values (new_rx and new_tx here: https://github.com/minhdanh/tmux-network-speed/pull/4/files#diff-5773c658da6edb4586ebfb5aed120f357c7aed943c7f100c32a4683c92a29d42R24-R25)
| case "$threshold_unit" in | ||
| "KB/s") | ||
| threshold_kb=$(echo "$threshold" | awk '{print $1}') | ||
| ;; | ||
| "MB/s") | ||
| threshold_kb=$(echo "$threshold * 1024" | bc -l) | ||
| ;; | ||
| "GB/s") | ||
| threshold_kb=$(echo "$threshold * 1048576" | bc -l) | ||
| ;; | ||
| *) | ||
| threshold_kb=$(echo "$threshold" | awk '{print $1}') | ||
| ;; | ||
| esac |
There was a problem hiding this comment.
If I understand this correctly, in case we don't set the threshold, then threshold_kb will be 0. Then the evaluation in line 115-117 always results to high_color being used:
if (($(echo "$speed_kb > $threshold_kb" | bc -l))); then
echo "$high_color"
else
Is that correct? I just want to make sure if the threshold is 0, then we use the default color.
|
|
||
| ### High-Speed Threshold and Color | ||
|
|
||
| You can set a threshold speed above which the color will change to indicate high speed. The default threshold is 1.0 MB/s. |
There was a problem hiding this comment.
This needs to be updated also I guess.
We don't want to have a default value for the threshold 😄
| set -g @network_speed_high_color '#[fg=red]' | ||
| ``` | ||
|
|
||
| ### High-Speed Threshold Unit |
There was a problem hiding this comment.
Just one minor thing, I think this section should be merged into "### High-Speed Threshold and Color" as they're both about the threshold and color.
|
Hi @Brams-s, I have added some comments several days ago. I really appreciate the effort and want this to be merged. I just wanted to check in to see if you have any questions or if there's anything I can do to help you complete the requested changes. Please let me know if you have any concerns or need further clarification on the changes I'm looking for. |
|
Hi @minhdanh, Sorry, was afk for a few days, thanks for the comments. I'll have a look at them later. |
… met