Skip to content

Conversation

CromFr
Copy link
Contributor

@CromFr CromFr commented Apr 21, 2020

I feel like having B/s at the end of the speed value is generally not necessary and takes too much space.

This pull request adds a udspeed-unit variable to be able to display the unit text in other formats. By default the value is B/s to keep the old behaviour. The K/M/G suffixes are still hardcoded and are displayed just before the unit.
Note: this is a "text-only" feature, setting speed-unit = "b/s won't change the output value from bytes per second to bits per second.

Examples:
speed-unit = "" will display speeds like: 128 42K 64G
speed-unit = "Bytes/sec" will display speeds like: 128 Bytes/sec 42KBytes/sec 64GBytes/sec

@codecov
Copy link

codecov bot commented Apr 21, 2020

Codecov Report

Merging #2068 into master will decrease coverage by 0.00%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #2068      +/-   ##
=========================================
- Coverage    6.92%   6.92%   -0.01%     
=========================================
  Files         153     153              
  Lines       10482   10483       +1     
=========================================
  Hits          726     726              
- Misses       9756    9757       +1     
Flag Coverage Δ
#unittests 6.92% <0.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
include/adapters/net.hpp 0.00% <ø> (ø)
src/adapters/net.cpp 0.00% <0.00%> (ø)
src/modules/network.cpp 0.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e309253...8e52cdc. Read the comment docs.

@kronn
Copy link
Contributor

kronn commented Oct 23, 2020

Would it make sense to rename the setting to speed-unit? The ud makes sense when you read the source, but feels like a typo otherwise :-)

@CromFr CromFr force-pushed the custom-network-speed-units branch from 604897b to 8e52cdc Compare October 23, 2020 22:21
@CromFr
Copy link
Contributor Author

CromFr commented Oct 23, 2020

The udspeed-unit name was chosen to match the other udspeed-minwidth setting, but since it is deprecated, it makes sense to change the name to speed-unit as you suggested :)

I also fixed the merge conflicts

Copy link
Member

@patrick96 patrick96 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Thanks @CromFr and thank you @kronn for the review.

@patrick96 patrick96 merged commit 50d8a1b into polybar:master Nov 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants