Add units support (POINT, PIXEL, SPACE) #2578

patrick96 · 2022-01-23T18:58:03Z

What type of PR is this? (check all applicable)

Description

This basically just resolves the merge conflicts in #2394, which itself just fixed the conflicts from #1671.

In this PR, I will try to bring this feature across the finishline, as I already discussed with @frebib some months ago on IRC.

Thanks to @Lomadriel and @frebib for all their efforts on this feature.

Checklist

Related Issues & Documents

Addresses the following feature requests:

Closes #1651
Closes #951

Fixes the following bugs:

Fixes #1700
Fixes #1265

Makes the following PRs obsolete:

Closes #1671
Closes #2394

Documentation (check all applicable)

This PR requires changes to the Wiki documentation (describe the changes)
This PR requires changes to the documentation inside the git repo (please add them to the PR).
Does not require documentation changes

We need to document that the %{O} tag now supports px and pt as units:

Offset `%{O}`

Inserts a gap of the specified size (can also be negative).

%{O10} pushed 10 pixels to the right %{O-5pt} 5 points to the left

The value specified can be any extent value.

The format-offset documentation should also be updated:

; Displace the format block horizontally by +/-N pixels or points
format-NAME-offset = N[pt|px]

We should also document the different supported formats for spacing and extends somewhere. Probably best to just add a section to the "Configuration" wiki page:

Sizes & Spacing

In many parts of the config, you can specify some kind of size (width, height, border) or spacing (margin, padding). These are very similar but all differ slightly. We differentiate between three types: spacing, extents, and "percentage with offset".

Spacing

Spacing can be specified as either a number of spaces (no unit), pixels (px) or points (pt).
For example:

; 10 spaces
label-padding = 10
; 20 pixels
label-padding = 20px
; 15 points
label-padding = 15pt

Spaces are just added as whitespace characters, the size depends on the active font.
One point is 1/72th of an inch and is translated to a number of pixels according to the specified DPI.

Extent

Extent values are almost like spacing but don't support spaces, only pixels (no unit or px) and points (pt).
These are mainly used to described the size of something (and thus can't support spaces)

Percentage with Offset

A percentage with offset specifies the size of something using a relative percentage together with an extent value that is added to it.
For example:

[bar/...]
...
width = 90%:-10pt

This specifies a bar width of 90% minus 10 points. What exactly the percentage is relative to depends on the setting. For the bar width, it is relative to the width of the monitor.

Both the percentage value and the extent value can also be specified on their own:

width = 100%
height = 12pt

We need to update the wiki to mention what kind of format is allowed in the following places:

Spacing

ramp-coreload-spacing in the CPU module
spacing in the FS module
padding and module-margin in the bar section
label padding and margin
format spacing, margin, padding

Extent

format-offset

Percentage with offset

Struts (in [global/wm])
Border size
Bar width/height
Bar offset (x/y)
tray-offset-*

- add a size_with_unit struct - add a geometry_format_values struct - move dpi initialisation from renderer.cpp to bar.cpp - add a string to size_with_unit converter - add point support (with pt) - add pixel support (with px)

The old names didn't really capture the purpose of the structs and function. space_type -> spacing_type space_size -> spacing_val size_type -> extent_type geometry -> extent_val geometry_format_values -> percentage_with_offset

No longer needed. The convert<spacing_val> function in config.cpp already does all the work for us and always setting the type to pixel was wrong. In addition, line-size should not be of type spacing_val but extent_val.

I tried to address most of my comments on the old PR

We can't just blindly add the x difference to the width because for example the width should increase if x < width and the increase keeps x < width. Similarly, we can't just add the offset to the width.

Co-authored-by: Patrick Ziegler <p.ziegler96@gmail.com>

codecov · 2022-01-23T19:01:28Z

Codecov Report

Merging #2578 (edc7ee2) into master (ab915fb) will increase coverage by 0.43%.
The diff coverage is 22.59%.

@@            Coverage Diff             @@
##           master    #2578      +/-   ##
==========================================
+ Coverage   11.37%   11.80%   +0.43%     
==========================================
  Files         153      153              
  Lines       11137    11248     +111     
==========================================
+ Hits         1267     1328      +61     
- Misses       9870     9920      +50

Flag	Coverage Δ
unittests	`11.80% <22.59%> (+0.43%)`	⬆️

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

Impacted Files	Coverage Δ
include/components/config.hpp	`0.00% <0.00%> (ø)`
include/components/controller.hpp	`0.00% <ø> (ø)`
include/components/renderer_interface.hpp	`100.00% <ø> (ø)`
include/components/types.hpp	`2.22% <0.00%> (-0.22%)`	⬇️
include/modules/fs.hpp	`0.00% <ø> (ø)`
include/modules/meta/base.hpp	`0.00% <ø> (ø)`
include/modules/meta/base.inl	`0.00% <0.00%> (ø)`
include/tags/parser.hpp	`95.23% <ø> (ø)`
include/tags/types.hpp	`100.00% <ø> (ø)`
src/components/bar.cpp	`0.00% <0.00%> (ø)`
... and 23 more

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 ab915fb...edc7ee2. Read the comment docs.

Negative spacing is never allowed and produces a config error. Extents allow negative values in theory, but only a few use-cases accept it. Only the extent value used for the `%{O}` tag and the offset value in percentage_with_offset can be negative. Everything else is capped below at 0. The final pixel value of percentage_with_offset also caps below at 0.

All changes preserve the existing semantics

Instead, we first check if the current tag is built, and only if it is, the spacing is prepended.

If there are two tags and the second one isn't built (module::build returns false), the space in between them is removed. For example in the mpd module: format-online = <toggle> <label-song> foo If mpd is not running, the mpd module will return false when trying to build the `<label-song>` tag. If we don't remove the space between `<toggle>` and `<label-song>`, we end up with two spaces between `<toggle>` and `foo`. This change is to match the old behavior where at least one trailing space character was removed from the builder.

patrick96 · 2022-02-20T20:10:09Z

Alright, this is finally done!!

I want to thank again @Lomadriel and @frebib for doing a lot of heavy lifting for this feature. This will be very useful to a lot of users.

Lomadriel and others added 21 commits August 1, 2021 18:13

add units support (POINT, PIXEL, SPACE) for polybar

4577858

- add a size_with_unit struct - add a geometry_format_values struct - move dpi initialisation from renderer.cpp to bar.cpp - add a string to size_with_unit converter - add point support (with pt) - add pixel support (with px)

Fix unit test compilation

d18af49

clang-format

88692f8

Better names

693d993

The old names didn't really capture the purpose of the structs and function. space_type -> spacing_type space_size -> spacing_val size_type -> extent_type geometry -> extent_val geometry_format_values -> percentage_with_offset

Remove parse_size_with_unit

fdc9c48

No longer needed. The convert<spacing_val> function in config.cpp already does all the work for us and always setting the type to pixel was wrong. In addition, line-size should not be of type spacing_val but extent_val.

Cleanup

efdc938

I tried to address most of my comments on the old PR

Fix renderer width calculation

42664d3

We can't just blindly add the x difference to the width because for example the width should increase if x < width and the increase keeps x < width. Similarly, we can't just add the offset to the width.

Rename geom_format_to_pixels to percentage_with_offset_to_pixel

9616bf9

Cleanup

d5de6c4

Apply suggested changes from Patrick on GitHub

7668780

Co-authored-by: Patrick Ziegler <p.ziegler96@gmail.com>

Update src/components/bar.cpp

7bdcf7f

Co-authored-by: Patrick Ziegler <p.ziegler96@gmail.com>

Update src/components/config.cpp

ca90065

Co-authored-by: Patrick Ziegler <p.ziegler96@gmail.com>

Update src/components/builder.cpp

06860ca

Co-authored-by: Patrick Ziegler <p.ziegler96@gmail.com>

Update src/components/builder.cpp

e8d4084

Co-authored-by: Patrick Ziegler <p.ziegler96@gmail.com>

config: Use stod for parsing percentage

077c040

Use stof instead of strtof

b25d6a8

units: Fix test edge cases

c93a4b1

Remove unnecessary clang-format toggle

0e0081e

Use percentage_with_offset for margin-{top,bottom}

7849a4e

Support negative extent values

08ff6d5

Merge remote-tracking branch 'upstream/master' into feat/units

4afaeee

patrick96 added the feature label Jan 23, 2022

patrick96 added this to the 3.6.0 milestone Jan 23, 2022

patrick96 self-assigned this Jan 23, 2022

patrick96 added 5 commits January 23, 2022 22:03

Rename unit to units and create a cpp file

ac13a0a

Merge remote-tracking branch 'upstream/master' into feat/units

e4c3a32

Move percentage_with_offset_to_pixel unit test to units

a1e0f2a

Add unit tests for units_utils

4d21396

patrick96 added 10 commits February 6, 2022 14:49

Fix parsing errors not being caught in config

281fda0

Print a proper error message for uncaught exceptions

c27e05d

Cleanup module::get_output

f77836d

All changes preserve the existing semantics

Stop using remove_trailing_space in module::get_output

793ee76

Instead, we first check if the current tag is built, and only if it is, the spacing is prepended.

Remove unused imports

f819d84

Merge remote-tracking branch 'upstream/master' into feat/units

880f768

Add changelog entry

9b80116

Remove unused setting

977ddac

Use percentage with offset for tray-offset

edc7ee2

patrick96 merged commit ce93188 into polybar:master Feb 20, 2022

patrick96 deleted the feat/units branch February 20, 2022 20:09

patrick96 mentioned this pull request Mar 3, 2022

fix: Allow negative values for tray and bar offset #2620

Merged

9 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Add units support (POINT, PIXEL, SPACE) #2578

Add units support (POINT, PIXEL, SPACE) #2578

Uh oh!

patrick96 commented Jan 23, 2022 •

edited

Loading

Uh oh!

codecov bot commented Jan 23, 2022 •

edited

Loading

Uh oh!

patrick96 commented Feb 20, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Add units support (POINT, PIXEL, SPACE) #2578

Add units support (POINT, PIXEL, SPACE) #2578

Uh oh!

Conversation

patrick96 commented Jan 23, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What type of PR is this? (check all applicable)

Description

Checklist

Related Issues & Documents

Documentation (check all applicable)

Offset %{O}

Sizes & Spacing

Spacing

Extent

Percentage with Offset

Uh oh!

codecov bot commented Jan 23, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

patrick96 commented Feb 20, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

patrick96 commented Jan 23, 2022 •

edited

Loading

Offset `%{O}`

codecov bot commented Jan 23, 2022 •

edited

Loading