Fix format-offset being ignored #2643
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Removes unused methods and fields
You cannot make polybar emit only the %{+o} tag without a color so that
codepath was unused.
The builder::offset function returned immediately if a valid extent was passed instead of when an invalid one was passed.
Codecov Report
@@ Coverage Diff @@
## hotfix/3.6.2 #2643 +/- ##
================================================
+ Coverage 12.03% 13.79% +1.75%
================================================
Files 153 153
Lines 11359 11266 -93
=================
8000
===============================
+ Hits 1367 1554 +187
+ Misses 9992 9712 -280
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
patrick96
referenced
this pull request
Mar 14, 2022
* add units support (POINT, PIXEL, SPACE) for polybar
- 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
* clang-format
* Better names
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
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
I tried to address most of my comments on the old PR
* Fix renderer width calculation
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
* Cleanup
* Apply suggested changes from Patrick on GitHub
Co-authored-by: Patrick Ziegler <p.ziegler96@gmail.com>
* Update src/components/bar.cpp
Co-authored-by: Patrick Ziegler <p.ziegler96@gmail.com>
* Update src/components/config.cpp
Co-authored-by: Patrick Ziegler <p.ziegler96@gmail.com>
* Update src/components/builder.cpp
Co-authored-by: Patrick Ziegler <p.ziegler96@gmail.com>
* Update src/components/builder.cpp
Co-authored-by: Patrick Ziegler <p.ziegler96@gmail.com>
* config: Use stod for parsing percentage
* Use stof instead of strtof
* units: Fix test edge cases
* Remove unnecessary clang-format toggle
* Use percentage_with_offset for margin-{top,bottom}
* Support negative extent values
* Rename unit to units and create a cpp file
* Move percentage_with_offset_to_pixel unit test to units
* Add unit tests for units_utils
* Clarify when and how negative spacing/extent is allowed
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.
* Fix parsing errors not being caught in config
* Print a proper error message for uncaught exceptions
* Cleanup module::get_output
All changes preserve the existing semantics
* Stop using remove_trailing_space in module::get_output
Instead, we first check if the current tag is built, and only if it is,
the spacing is prepended.
* Remove unused imports
* Restore old behavior
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 modul
8199
e:
format- <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.
* Add changelog entry
* Remove unused setting
* Use percentage with offset for tray-offset
Co-authored-by: Jérôme BOULMIER <jerome.boulmier@outlook.fr>
Co-authored-by: Joe Groocock <github@frebib.net>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What type of PR is this? (check all applicable)
Description
The code checked if the offset was non-zero and then returned instead of returning when the offset was zero.
This also adds tests for the builder.
There was also some undefined behavior where the rvalue returned by
bar::settings()was taken by reference. Now, it returns a reference to the bar settings instead of a copy.Related Issues & Documents
Reported in a comment: ce93188#commitcomment-68605346
Documentation (check all applicable)