fix(builder): ignored offsets

The builder::offset function returned immediately if a valid extent was
passed instead of when an invalid one was passed.
This commit is contained in:
patrick96 2022-03-14 22:11:46 +01:00 committed by Patrick Ziegler
parent 6fdc3114f3
commit e904b408d1
14 changed files with 266 additions and 114 deletions

View File

@ -1,12 +1,12 @@
#pragma once
#include <pulse/pulseaudio.h>
#include <queue>
#include "common.hpp"
#include "settings.hpp"
#include "errors.hpp"
#include "settings.hpp"
#include "utils/math.hpp"
// fwd
struct pa_context;
@ -46,21 +46,21 @@ class pulseaudio {
bool is_muted();
private:
void update_volume(pa_operation *o);
static void check_mute_callback(pa_context *context, const pa_sink_info *info, int eol, void *userdata);
static void get_sink_volume_callback(pa_context *context, const pa_sink_info *info, int is_last, void *userdata);
void update_volume(pa_operation* o);
static void check_mute_callback(pa_context* context, const pa_sink_info* info, int eol, void* userdata);
static void get_sink_volume_callback(pa_context* context, const pa_sink_info* info, int is_last, void* userdata);
static void subscribe_callback(pa_context* context, pa_subscription_event_type_t t, uint32_t idx, void* userdata);
static void simple_callback(pa_context *context, int success, void *userdata);
static void sink_info_callback(pa_context *context, const pa_sink_info *info, int eol, void *userdata);
static void context_state_callback(pa_context *context, void *userdata);
static void simple_callback(pa_context* context, int success, void* userdata);
static void sink_info_callback(pa_context* context, const pa_sink_info* info, int eol, void* userdata);
static void context_state_callback(pa_context* context, void* userdata);
inline void wait_loop(pa_operation *op, pa_threaded_mainloop *loop);
inline void wait_loop(pa_operation* op, pa_threaded_mainloop* loop);
const logger& m_log;
// used for temporary callback results
int success{0};
pa_cvolume cv;
pa_cvolume cv{};
bool muted{false};
// default sink name
static constexpr auto DEFAULT_SINK = "@DEFAULT_SINK@";

View File

@ -46,7 +46,7 @@ class bar : public xpp::event::sink<evt::button_press, evt::expose, evt::propert
bool only_initialize_values);
~bar();
const bar_settings settings() const;
const bar_settings& settings() const;
void start();

View File

@ -31,8 +31,8 @@ class builder {
void font_close();
void background(rgba color);
void background_close();
void color(rgba color);
void color_close();
void foreground(rgba color);
void foreground_close();
void overline(const rgba& color);
void overline_close();
void underline(const rgba& color);
@ -44,7 +44,7 @@ class builder {
void action(mousebtn btn, const modules::module_interface& module, string action, string data, const label_t& label);
void action_close();
static string get_spacing_format_string(const spacing_val& space);
static string get_spacing_format_string(spacing_val space);
protected:
void append(const string& text);
@ -58,7 +58,7 @@ class builder {
void tag_close(tags::attribute attr);
private:
const bar_settings m_bar;
const bar_settings& m_bar;
string m_output;
map<tags::syntaxtag, int> m_tags{};

View File

@ -142,7 +142,7 @@ namespace modules {
template <class Impl>
class module : public module_interface {
public:
module(const bar_settings bar, string name);
module(const bar_settings& bar, string name);
~module() noexcept;
static constexpr auto EVENT_MODULE_TOGGLE = "module_toggle";
@ -197,7 +197,7 @@ namespace modules {
protected:
signal_emitter& m_sig;
const bar_settings m_bar;
const bar_settings& m_bar;
const logger& m_log;
const config& m_conf;

View File

@ -15,7 +15,7 @@ namespace modules {
// module<Impl> public {{{
template <typename Impl>
module<Impl>::module(const bar_settings bar, string name)
module<Impl>::module(const bar_settings& bar, string name)
: m_sig(signal_emitter::make())
, m_bar(bar)
, m_log(logger::make())

View File

@ -27,6 +27,8 @@ namespace units_utils {
spacing_type parse_spacing_unit(const string& str);
spacing_val parse_spacing(const string& str);
extent_val spacing_to_extent(spacing_val spacing);
} // namespace units_utils
POLYBAR_NS_END

View File

@ -1,4 +1,5 @@
#include "adapters/pulseaudio.hpp"
#include "components/logger.hpp"
POLYBAR_NS
@ -6,7 +7,8 @@ POLYBAR_NS
/**
* Construct pulseaudio object
*/
pulseaudio::pulseaudio(const logger& logger, string&& sink_name, bool max_volume) : m_log(logger), spec_s_name(sink_name) {
pulseaudio::pulseaudio(const logger& logger, string&& sink_name, bool max_volume)
: m_log(logger), spec_s_name(sink_name) {
m_mainloop = pa_threaded_mainloop_new();
if (!m_mainloop) {
throw pulseaudio_error("Could not create pulseaudio threaded mainloop.");
@ -50,7 +52,7 @@ pulseaudio::pulseaudio(const logger& logger, string&& sink_name, bool max_volume
throw pulseaudio_error("Could not connect to pulseaudio server.");
}
pa_operation *op{nullptr};
pa_operation* op{nullptr};
if (!sink_name.empty()) {
op = pa_context_get_sink_info_by_name(m_context, sink_name.c_str(), sink_info_callback, this);
wait_loop(op, m_mainloop);
@ -69,14 +71,14 @@ pulseaudio::pulseaudio(const logger& logger, string&& sink_name, bool max_volume
auto event_types = static_cast<pa_subscription_mask_t>(PA_SUBSCRIPTION_MASK_SINK | PA_SUBSCRIPTION_MASK_SERVER);
op = pa_context_subscribe(m_context, event_types, simple_callback, this);
wait_loop(op, m_mainloop);
if (!success)
if (!success) {
throw pulseaudio_error("Failed to subscribe to sink.");
}
pa_context_set_subscribe_callback(m_context, subscribe_callback, this);
update_volume(op);
pa_threaded_mainloop_unlock(m_mainloop);
}
/**
@ -87,7 +89,6 @@ pulseaudio::~pulseaudio() {
pa_context_disconnect(m_context);
pa_context_unref(m_context);
pa_threaded_mainloop_free(m_mainloop);
}
/**
@ -110,7 +111,7 @@ bool pulseaudio::wait() {
int pulseaudio::process_events() {
int ret = m_events.size();
pa_threaded_mainloop_lock(m_mainloop);
pa_operation *o{nullptr};
pa_operation* o{nullptr};
// clear the queue
while (!m_events.empty()) {
switch (m_events.front()) {
@ -133,8 +134,9 @@ int pulseaudio::process_events() {
case evtype::REMOVE:
o = pa_context_get_sink_info_by_name(m_context, DEFAULT_SINK, sink_info_callback, this);
wait_loop(o, m_mainloop);
if (spec_s_name != s_name)
if (spec_s_name != s_name) {
m_log.notice("pulseaudio: using default sink %s", s_name);
}
break;
default:
break;
@ -168,7 +170,7 @@ void pulseaudio::set_volume(float percentage) {
pa_threaded_mainloop_lock(m_mainloop);
pa_volume_t vol = math_util::percentage_to_value<pa_volume_t>(percentage, PA_VOLUME_MUTED, PA_VOLUME_NORM);
pa_cvolume_scale(&cv, vol);
pa_operation *op = pa_context_set_sink_volume_by_index(m_context, m_index, &cv, simple_callback, this);
pa_operation* op = pa_context_set_sink_volume_by_index(m_context, m_index, &cv, simple_callback, this);
wait_loop(op, m_mainloop);
if (!success)
throw pulseaudio_error("Failed to set sink volume.");
@ -193,7 +195,7 @@ void pulseaudio::inc_volume(int delta_perc) {
}
} else
pa_cvolume_dec(&cv, vol);
pa_operation *op = pa_context_set_sink_volume_by_index(m_context, m_index, &cv, simple_callback, this);
pa_operation* op = pa_context_set_sink_volume_by_index(m_context, m_index, &cv, simple_callback, this);
wait_loop(op, m_mainloop);
if (!success)
throw pulseaudio_error("Failed to set sink volume.");
@ -205,7 +207,7 @@ void pulseaudio::inc_volume(int delta_perc) {
*/
void pulseaudio::set_mute(bool mode) {
pa_threaded_mainloop_lock(m_mainloop);
pa_operation *op = pa_context_set_sink_mute_by_index(m_context, m_index, mode, simple_callback, this);
pa_operation* op = pa_context_set_sink_mute_by_index(m_context, m_index, mode, simple_callback, this);
wait_loop(op, m_mainloop);
if (!success)
throw pulseaudio_error("Failed to mute sink.");
@ -229,7 +231,7 @@ bool pulseaudio::is_muted() {
/**
* Update local volume cache
*/
void pulseaudio::update_volume(pa_operation *o) {
void pulseaudio::update_volume(pa_operation* o) {
o = pa_context_get_sink_info_by_index(m_context, m_index, get_sink_volume_callback, this);
wait_loop(o, m_mainloop);
}
@ -237,8 +239,8 @@ void pulseaudio::update_volume(pa_operation *o) {
/**
* Callback when getting volume
*/
void pulseaudio::get_sink_volume_callback(pa_context *, const pa_sink_info *info, int, void *userdata) {
pulseaudio* This = static_cast<pulseaudio *>(userdata);
void pulseaudio::get_sink_volume_callback(pa_context*, const pa_sink_info* info, int, void* userdata) {
pulseaudio* This = static_cast<pulseaudio*>(userdata);
if (info) {
This->cv = info->volume;
This->muted = info->mute;
@ -249,18 +251,18 @@ void pulseaudio::get_sink_volume_callback(pa_context *, const pa_sink_info *info
/**
* Callback when subscribing to changes
*/
void pulseaudio::subscribe_callback(pa_context *, pa_subscription_event_type_t t, uint32_t idx, void* userdata) {
pulseaudio *This = static_cast<pulseaudio *>(userdata);
switch(t & PA_SUBSCRIPTION_EVENT_FACILITY_MASK) {
void pulseaudio::subscribe_callback(pa_context*, pa_subscription_event_type_t t, uint32_t idx, void* userdata) {
pulseaudio* This = static_cast<pulseaudio*>(userdata);
switch (t & PA_SUBSCRIPTION_EVENT_FACILITY_MASK) {
case PA_SUBSCRIPTION_EVENT_SERVER:
switch(t & PA_SUBSCRIPTION_EVENT_TYPE_MASK) {
switch (t & PA_SUBSCRIPTION_EVENT_TYPE_MASK) {
case PA_SUBSCRIPTION_EVENT_CHANGE:
This->m_events.emplace(evtype::SERVER);
break;
}
break;
case PA_SUBSCRIPTION_EVENT_SINK:
switch(t & PA_SUBSCRIPTION_EVENT_TYPE_MASK) {
switch (t & PA_SUBSCRIPTION_EVENT_TYPE_MASK) {
case PA_SUBSCRIPTION_EVENT_NEW:
This->m_events.emplace(evtype::NEW);
break;
@ -281,18 +283,17 @@ void pulseaudio::subscribe_callback(pa_context *, pa_subscription_event_type_t t
/**
* Simple callback to check for success
*/
void pulseaudio::simple_callback(pa_context *, int success, void *userdata) {
pulseaudio *This = static_cast<pulseaudio *>(userdata);
void pulseaudio::simple_callback(pa_context*, int success, void* userdata) {
pulseaudio* This = static_cast<pulseaudio*>(userdata);
This->success = success;
pa_threaded_mainloop_signal(This->m_mainloop, 0);
}
/**
* Callback when getting sink info & existence
*/
void pulseaudio::sink_info_callback(pa_context *, const pa_sink_info *info, int eol, void *userdata) {
pulseaudio *This = static_cast<pulseaudio *>(userdata);
void pulseaudio::sink_info_callback(pa_context*, const pa_sink_info* info, int eol, void* userdata) {
pulseaudio* This = static_cast<pulseaudio*>(userdata);
if (!eol && info) {
This->m_index = info->index;
This->s_name = info->name;
@ -303,8 +304,8 @@ void pulseaudio::sink_info_callback(pa_context *, const pa_sink_info *info, int
/**
* Callback when context state changes
*/
void pulseaudio::context_state_callback(pa_context *context, void *userdata) {
pulseaudio* This = static_cast<pulseaudio *>(userdata);
void pulseaudio::context_state_callback(pa_context* context, void* userdata) {
pulseaudio* This = static_cast<pulseaudio*>(userdata);
switch (pa_context_get_state(context)) {
case PA_CONTEXT_READY:
case PA_CONTEXT_TERMINATED:
@ -320,9 +321,10 @@ void pulseaudio::context_state_callback(pa_context *context, void *userdata) {
}
}
inline void pulseaudio::wait_loop(pa_operation *op, pa_threaded_mainloop *loop) {
while (pa_operation_get_state(op) != PA_OPERATION_DONE)
inline void pulseaudio::wait_loop(pa_operation* op, pa_threaded_mainloop* loop) {
while (pa_operation_get_state(op) != PA_OPERATION_DONE) {
pa_threaded_mainloop_wait(loop);
}
pa_operation_unref(op);
}

View File

@ -361,7 +361,7 @@ bar::~bar() {
/**
* Get the bar settings container
*/
const bar_settings bar::settings() const {
const bar_settings& bar::settings() const {
return m_opts;
}

View File

@ -46,7 +46,7 @@ void builder::reset() {
*/
string builder::flush() {
background_close();
color_close();
foreground_close();
font_close();
overline_color_close();
underline_color_close();
@ -122,7 +122,7 @@ void builder::node(const label_t& label) {
background(label->m_background);
}
if (label->m_foreground.has_color()) {
color(label->m_foreground);
foreground(label->m_foreground);
}
if (label->m_padding.left) {
@ -139,7 +139,7 @@ void builder::node(const label_t& label) {
background_close();
}
if (label->m_foreground.has_color()) {
color_close();
foreground_close();
}
if (label->m_underline.has_color()) {
@ -164,6 +164,7 @@ void builder::node_repeat(const label_t& label, size_t n) {
while (n--) {
text += label_text;
}
label_t tmp{new label_t::element_type{text}};
tmp->replace_defined_values(label);
node(tmp);
@ -173,7 +174,7 @@ void builder::node_repeat(const label_t& label, size_t n) {
* Insert tag that will offset the contents by the given extent
*/
void builder::offset(extent_val extent) {
if (extent) {
if (!extent) {
return;
}
tag_open(syntaxtag::O, units_utils::extent_to_string(extent));
@ -231,7 +232,7 @@ void builder::background_close() {
/**
* Insert tag to alter the current foreground color
*/
void builder::color(rgba color) {
void builder::foreground(rgba color) {
color = color.try_apply_alpha_to(m_bar.foreground);
auto hex = color_util::simplify_hex(color);
@ -241,7 +242,7 @@ void builder::color(rgba color) {
/**
* Insert tag to reset the foreground color
*/
void builder::color_close() {
void builder::foreground_close() {
tag_close(syntaxtag::F);
}
@ -305,7 +306,7 @@ void builder::control(controltag tag) {
str = "R";
break;
default:
break;
throw runtime_error("Invalid controltag: " + to_string(to_integral(tag)));
}
if (!str.empty()) {
@ -321,7 +322,7 @@ void builder::control(controltag tag) {
void builder::action(mousebtn index, string action) {
if (!action.empty()) {
action = string_util::replace_all(action, ":", "\\:");
tag_open(syntaxtag::A, to_string(static_cast<int>(index)) + ":" + action + ":");
tag_open(syntaxtag::A, to_string(to_integral(index)) + ":" + action + ":");
}
}
@ -401,6 +402,8 @@ void builder::tag_open(syntaxtag tag, const string& value) {
case syntaxtag::r:
append("%{r}");
break;
default:
throw runtime_error("Invalid tag: " + to_string(to_integral(tag)));
}
}
@ -415,14 +418,14 @@ void builder::tag_open(attribute attr) {
m_attrs[attr] = true;
switch (attr) {
case attribute::NONE:
break;
case attribute::UNDERLINE:
append("%{+u}");
break;
case attribute::OVERLINE:
append("%{+o}");
break;
default:
throw runtime_error("Invalid attribute: " + to_string(to_integral(attr)));
}
}
@ -471,44 +474,28 @@ void builder::tag_close(attribute attr) {
m_attrs[attr] = false;
switch (attr) {
case attribute::NONE:
break;
case attribute::UNDERLINE:
append("%{-u}");
break;
case attribute::OVERLINE:
append("%{-o}");
break;
default:
throw runtime_error("Invalid attribute: " + to_string(to_integral(attr)));
}
}
string builder::get_spacing_format_string(const spacing_val& space) {
string builder::get_spacing_format_string(spacing_val space) {
float value = space.value;
if (value == 0) {
return "";
}
string out;
if (space.type == spacing_type::SPACE) {
out += string(value, ' ');
return string(value, ' ');
} else {
out += "%{O";
switch (space.type) {
case spacing_type::POINT:
out += to_string(value) + "pt";
break;
case spacing_type::PIXEL:
out += to_string(static_cast<int>(value)) + "px";
break;
default:
break;
return "%{O" + units_utils::extent_to_string(units_utils::spacing_to_extent(space)) + "}";
}
out += '}';
}
return out;
}
POLYBAR_NS_END

View File

@ -25,7 +25,7 @@ namespace modules {
builder->background(bg);
}
if (fg.has_color()) {
builder->color(fg);
builder->foreground(fg);
}
if (ul.has_color()) {
builder->underline(ul);
@ -46,7 +46,7 @@ namespace modules {
builder->background(bg);
}
if (fg.has_color()) {
builder->color(fg);
builder->foreground(fg);
}
if (ul.has_color()) {
builder->underline(ul);
@ -71,7 +71,7 @@ namespace modules {
builder->underline_close();
}
if (fg.has_color()) {
builder->color_close();
builder->foreground_close();
}
if (bg.has_color()) {
builder->background_close();

View File

@ -58,8 +58,9 @@ namespace modules {
bool pulseaudio_module::has_event() {
// Poll for mixer and control events
try {
if (m_pulseaudio->wait())
if (m_pulseaudio->wait()) {
return true;
}
} catch (const pulseaudio_error& e) {
m_log.err("%s: %s", name(), e.what());
}

View File

@ -135,6 +135,23 @@ namespace units_utils {
return {type, size_value};
}
extent_val spacing_to_extent(spacing_val spacing) {
extent_type t;
switch (spacing.type) {
case spacing_type::POINT:
t = extent_type::POINT;
break;
case spacing_type::PIXEL:
t = extent_type::PIXEL;
break;
default:
throw std::runtime_error("spacing_to_extent: Illegal type: " + to_string(to_integral(spacing.type)));
}
return {t, spacing.value};
}
} // namespace units_utils
POLYBAR_NS_END

View File

@ -58,6 +58,7 @@ add_unit_test(utils/string)
add_unit_test(utils/file)
add_unit_test(utils/process)
add_unit_test(utils/units)
add_unit_test(components/builder)
add_unit_test(components/command_line)
add_unit_test(components/config_parser)
add_unit_test(drawtypes/label)

View File

@ -0,0 +1,142 @@
#include "components/builder.hpp"
#include "common/test.hpp"
using namespace polybar;
static const rgba FG = rgba("#00FF00");
static const rgba BG = rgba("#FF0000");
class BuilderTest : public ::testing::Test {
protected:
virtual void SetUp() override {
opts.foreground = FG;
opts.background = BG;
opts.spacing = ZERO_SPACE;
}
bar_settings opts{};
builder b{opts};
};
TEST_F(BuilderTest, empty) {
EXPECT_EQ("", b.flush());
}
TEST_F(BuilderTest, text) {
b.node("foo");
EXPECT_EQ("foo", b.flush());
}
TEST_F(BuilderTest, textFont) {
b.node("foo", 12);
EXPECT_EQ("%{T12}foo%{T-}", b.flush());
}
using offset_test = std::pair<extent_val, string>;
class OffsetTest : public BuilderTest, public ::testing::WithParamInterface<offset_test> {};
vector<offset_test> offset_test_list = {
{ZERO_PX_EXTENT, ""},
{{extent_type::POINT, 0}, ""},
{{extent_type::PIXEL, 10}, "%{O10px}"},
{{extent_type::PIXEL, -10}, "%{O-10px}"},
{{extent_type::POINT, 23}, "%{O23pt}"},
{{extent_type::POINT, -1}, "%{O-1pt}"},
};
INSTANTIATE_TEST_SUITE_P(Inst, OffsetTest, ::testing::ValuesIn(offset_test_list));
TEST_P(OffsetTest, correctness) {
b.offset(GetParam().first);
EXPECT_EQ(GetParam().second, b.flush());
}
using spacing_test = std::pair<spacing_val, string>;
class SpacingTest : public BuilderTest, public ::testing::WithParamInterface<spacing_test> {};
vector<spacing_test> spacing_test_list = {
{ZERO_SPACE, ""},
{{spacing_type::SPACE, 2}, " "},
{{spacing_type::PIXEL, 3}, "%{O3px}"},
{{spacing_type::POINT, 4}, "%{O4pt}"},
};
INSTANTIATE_TEST_SUITE_P(Inst, SpacingTest, ::testing::ValuesIn(spacing_test_list));
TEST_P(SpacingTest, correctness) {
b.spacing(GetParam().first);
EXPECT_EQ(GetParam().second, b.flush());
}
TEST_P(SpacingTest, get_spacing_format_string) {
b.spacing(GetParam().first);
EXPECT_EQ(GetParam().second, b.flush());
}
TEST_F(BuilderTest, tags) {
b.font(10);
b.font_close();
EXPECT_EQ("%{T10}%{T-}", b.flush());
b.background(rgba("#f0f0f0"));
b.background_close();
EXPECT_EQ("%{B#f0f0f0}%{B-}", b.flush());
b.foreground(rgba("#0f0f0f"));
b.foreground_close();
EXPECT_EQ("%{F#0f0f0f}%{F-}", b.flush());
b.overline(rgba("#0e0e0e"));
b.overline_close();
// Last tag is from auto-closing during flush
EXPECT_EQ("%{o#0e0e0e}%{+o}%{-o}%{o-}", b.flush());
b.underline(rgba("#0d0d0d"));
b.underline_close();
// Last tag is from auto-closing during flush
EXPECT_EQ("%{u#0d0d0d}%{+u}%{-u}%{u-}", b.flush());
b.control(tags::controltag::R);
EXPECT_EQ("%{PR}", b.flush());
b.action(mousebtn::LEFT, "cmd");
b.action_close();
EXPECT_EQ("%{A1:cmd:}%{A}", b.flush());
}
TEST_F(BuilderTest, tagsAutoClose) {
b.font(20);
EXPECT_EQ("%{T20}%{T-}", b.flush());
b.background(rgba("#f0f0f0"));
EXPECT_EQ("%{B#f0f0f0}%{B-}", b.flush());
b.foreground(rgba("#0f0f0f"));
EXPECT_EQ("%{F#0f0f0f}%{F-}", b.flush());
b.overline(rgba("#0e0e0e"));
EXPECT_EQ("%{o#0e0e0e}%{+o}%{o-}%{-o}", b.flush());
b.underline(rgba("#0d0d0d"));
EXPECT_EQ("%{u#0d0d0d}%{+u}%{u-}%{-u}", b.flush());
b.action(mousebtn::LEFT, "cmd");
EXPECT_EQ("%{A1:cmd:}%{A}", b.flush());
}
TEST_F(BuilderTest, invalidTags) {
EXPECT_THROW(b.control(tags::controltag::NONE), std::runtime_error);
}
TEST_F(BuilderTest, colorBlending) {
b.background(rgba(0x12000000, rgba::type::ALPHA_ONLY));
b.background_close();
EXPECT_EQ("%{B#12ff0000}%{B-}", b.flush());
b.foreground(rgba(0x34000000, rgba::type::ALPHA_ONLY));
b.foreground_close();
EXPECT_EQ("%{F#3400ff00}%{F-}", b.flush());
}