diff --git a/CHANGELOG.md b/CHANGELOG.md index 40363f2e..4664eb3e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -224,6 +224,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Some modules stop updating when system time moves backwards. ([`#857`](https://github.com/polybar/polybar/issues/857), [`#1932`](https://github.com/polybar/polybar/issues/1932)) - Broken positioning in Openbox when the bar is hidden and shown again ([`#2021`](https://github.com/polybar/polybar/issues/2021)) +- Handling of action blocks that contain negative offsets + ([`#1814`](https://github.com/polybar/polybar/issues/1814)) ## [3.5.7] - 2021-09-21 ### Fixed diff --git a/include/tags/action_context.hpp b/include/tags/action_context.hpp index 504e4209..533316ed 100644 --- a/include/tags/action_context.hpp +++ b/include/tags/action_context.hpp @@ -40,6 +40,8 @@ namespace tags { /** * End position of the action block (exclusive), relative to the alignment. + * + * May be incrementally updated while the action block is still open */ double end_x{0}; mousebtn button; @@ -80,6 +82,20 @@ namespace tags { action_t action_open(mousebtn btn, const string&& cmd, alignment align, double x); std::pair action_close(mousebtn btn, alignment align, double x); + /** + * Compensate for the current position moving backwards in the renderer. + * + * This can happen if negative offsets are used. + * + * The policy is that an action block's start is the smallest x position observed while the block is open. + * And its end is the largest x position observed while it is open. + * + * @param a The current alignment + * @param old_x The x position before the backwards move + * @param new_x The x position after the backwards move new_x < old_x + */ + void compensate_for_negative_move(alignment a, double old_x, double new_x); + void set_alignment_start(const alignment a, const double x); std::map get_actions(int x) const; @@ -115,6 +131,6 @@ namespace tags { {alignment::NONE, 0}, {alignment::LEFT, 0}, {alignment::CENTER, 0}, {alignment::RIGHT, 0}}; }; -} // namespace tags +} // namespace tags POLYBAR_NS_END diff --git a/include/tags/dispatch.hpp b/include/tags/dispatch.hpp index 2ceccbaf..3e9453b5 100644 --- a/include/tags/dispatch.hpp +++ b/include/tags/dispatch.hpp @@ -31,6 +31,7 @@ namespace tags { protected: void handle_text(renderer_interface& renderer, string&& data); void handle_action(renderer_interface& renderer, mousebtn btn, bool closing, const string&& cmd); + void handle_offset(renderer_interface& renderer, extent_val offset); void handle_control(controltag ctrl); private: @@ -39,6 +40,6 @@ namespace tags { unique_ptr m_ctxt; action_context& m_action_ctxt; }; -} // namespace tags +} // namespace tags POLYBAR_NS_END diff --git a/src/tags/action_context.cpp b/src/tags/action_context.cpp index d6993080..88479a0d 100644 --- a/src/tags/action_context.cpp +++ b/src/tags/action_context.cpp @@ -37,7 +37,28 @@ namespace tags { } void action_context::set_end(action_t id, double x) { - m_action_blocks[id].end_x = x; + /* + * Only ever increase the end position. + * A larger end position may have been set before. + */ + m_action_blocks[id].end_x = std::max(m_action_blocks[id].end_x, x); + } + + void action_context::compensate_for_negative_move(alignment a, double old_x, double new_x) { + assert(new_x < old_x); + for (auto& block : m_action_blocks) { + if (block.is_open && block.align == a) { + // Move back the start position if a smaller position is observed + if (block.start_x > new_x) { + block.start_x = new_x; + } + + // Move forward the end position if a larger position is observed + if (old_x > block.end_x) { + block.end_x = old_x; + } + } + } } void action_context::set_alignment_start(const alignment a, const double x) { @@ -104,6 +125,6 @@ namespace tags { const std::vector& action_context::get_blocks() const { return m_action_blocks; } -} // namespace tags +} // namespace tags POLYBAR_NS_END diff --git a/src/tags/dispatch.cpp b/src/tags/dispatch.cpp index 5a12bd8f..0a1a3f43 100644 --- a/src/tags/dispatch.cpp +++ b/src/tags/dispatch.cpp @@ -42,6 +42,9 @@ namespace tags { continue; } + alignment old_alignment = m_ctxt->get_alignment(); + double old_x = old_alignment == alignment::NONE ? 0 : renderer.get_x(*m_ctxt); + if (el.is_tag) { switch (el.tag_data.type) { case tags::tag_type::FORMAT: @@ -59,7 +62,7 @@ namespace tags { m_ctxt->apply_font(el.tag_data.font); break; case tags::syntaxtag::O: - renderer.render_offset(*m_ctxt, el.tag_data.offset); + handle_offset(renderer, el.tag_data.offset); break; case tags::syntaxtag::R: m_ctxt->apply_reverse(); @@ -97,6 +100,13 @@ namespace tags { } else { handle_text(renderer, std::move(el.data)); } + + if (old_alignment == m_ctxt->get_alignment()) { + double new_x = renderer.get_x(*m_ctxt); + if (new_x < old_x) { + m_action_ctxt.compensate_for_negative_move(old_alignment, old_x, new_x); + } + } } /* @@ -136,6 +146,10 @@ namespace tags { } } + void dispatch::handle_offset(renderer_interface& renderer, extent_val offset) { + renderer.render_offset(*m_ctxt, offset); + } + void dispatch::handle_control(controltag ctrl) { switch (ctrl) { case controltag::R: @@ -146,6 +160,6 @@ namespace tags { } } -} // namespace tags +} // namespace tags POLYBAR_NS_END diff --git a/tests/unit_tests/tags/dispatch.cpp b/tests/unit_tests/tags/dispatch.cpp index b8d4738b..309e4002 100644 --- a/tests/unit_tests/tags/dispatch.cpp +++ b/tests/unit_tests/tags/dispatch.cpp @@ -19,7 +19,7 @@ namespace polybar { inline bool operator==(const extent_val& a, const extent_val& b) { return a.type == b.type && a.value == b.value; } -} // namespace polybar +} // namespace polybar class MockRenderer : public renderer_interface { public: @@ -135,7 +135,19 @@ TEST_F(DispatchTest, unclosedActions) { TEST_F(DispatchTest, actions) { { InSequence seq; + // Called for 'foo' + EXPECT_CALL(r, get_x(_)).WillOnce(Return(0)); EXPECT_CALL(r, get_x(_)).WillOnce(Return(3)); + // Called for '%{A1:cmd:}' + EXPECT_CALL(r, get_x(_)).WillOnce(Return(3)); + EXPECT_CALL(r, get_x(_)).WillOnce(Return(3)); + EXPECT_CALL(r, get_x(_)).WillOnce(Return(3)); + // Called for 'bar' + EXPECT_CALL(r, get_x(_)).WillOnce(Return(3)); + EXPECT_CALL(r, get_x(_)).WillOnce(Return(6)); + // Called for '%{A}' + EXPECT_CALL(r, get_x(_)).WillOnce(Return(6)); + EXPECT_CALL(r, get_x(_)).WillOnce(Return(6)); EXPECT_CALL(r, get_x(_)).WillOnce(Return(6)); } @@ -154,3 +166,123 @@ TEST_F(DispatchTest, actions) { EXPECT_EQ(mousebtn::LEFT, blk.button); EXPECT_EQ("cmd", blk.cmd); } + +TEST_F(DispatchTest, actionOffsetStart) { + { + InSequence seq; + // Called for 'a' + EXPECT_CALL(r, get_x(_)).WillOnce(Return(0)); + EXPECT_CALL(r, get_x(_)).WillOnce(Return(1)); + // Called for '%{A1:cmd:}' + EXPECT_CALL(r, get_x(_)).WillOnce(Return(1)); + EXPECT_CALL(r, get_x(_)).WillOnce(Return(1)); + EXPECT_CALL(r, get_x(_)).WillOnce(Return(1)); + // Called for '%{O-1}' + EXPECT_CALL(r, get_x(_)).WillOnce(Return(1)); + EXPECT_CALL(r, get_x(_)).WillOnce(Return(0)); + // Called for 'bar' + EXPECT_CALL(r, get_x(_)).WillOnce(Return(0)); + EXPECT_CALL(r, get_x(_)).WillOnce(Return(3)); + // Called for '%{A}' + EXPECT_CALL(r, get_x(_)).WillOnce(Return(3)); + EXPECT_CALL(r, get_x(_)).WillOnce(Return(3)); + EXPECT_CALL(r, get_x(_)).WillOnce(Return(3)); + } + + bar_settings settings; + m_dispatch->parse(settings, r, "%{l}a%{A1:cmd:}%{O-1}bar%{A}"); + + const auto& actions = m_action_ctxt->get_blocks(); + + ASSERT_EQ(1, actions.size()); + + const auto& blk = actions[0]; + + EXPECT_EQ(0, blk.start_x); + EXPECT_EQ(3, blk.end_x); + EXPECT_EQ(alignment::LEFT, blk.align); + EXPECT_EQ(mousebtn::LEFT, blk.button); + EXPECT_EQ("cmd", blk.cmd); +} + +TEST_F(DispatchTest, actionOffsetEnd) { + { + InSequence seq; + // Called for 'a' + EXPECT_CALL(r, get_x(_)).WillOnce(Return(0)); + EXPECT_CALL(r, get_x(_)).WillOnce(Return(1)); + // Called for '%{A1:cmd:}' + EXPECT_CALL(r, get_x(_)).WillOnce(Return(1)); + EXPECT_CALL(r, get_x(_)).WillOnce(Return(1)); + EXPECT_CALL(r, get_x(_)).WillOnce(Return(1)); + // Called for 'bar' + EXPECT_CALL(r, get_x(_)).WillOnce(Return(1)); + EXPECT_CALL(r, get_x(_)).WillOnce(Return(4)); + // Called for '%{O-3}' + EXPECT_CALL(r, get_x(_)).WillOnce(Return(4)); + EXPECT_CALL(r, get_x(_)).WillOnce(Return(1)); + // Called for '%{A}' + EXPECT_CALL(r, get_x(_)).WillOnce(Return(1)); + EXPECT_CALL(r, get_x(_)).WillOnce(Return(1)); + EXPECT_CALL(r, get_x(_)).WillOnce(Return(1)); + } + + bar_settings settings; + m_dispatch->parse(settings, r, "%{l}a%{A1:cmd:}bar%{O-3}%{A}"); + + const auto& actions = m_action_ctxt->get_blocks(); + + ASSERT_EQ(1, actions.size()); + + const auto& blk = actions[0]; + + EXPECT_EQ(1, blk.start_x); + EXPECT_EQ(4, blk.end_x); + EXPECT_EQ(alignment::LEFT, blk.align); + EXPECT_EQ(mousebtn::LEFT, blk.button); + EXPECT_EQ("cmd", blk.cmd); +} + +TEST_F(DispatchTest, actionOffsetBefore) { + { + InSequence seq; + // Called for 'a' + EXPECT_CALL(r, get_x(_)).WillOnce(Return(0)); + EXPECT_CALL(r, get_x(_)).WillOnce(Return(1)); + // Called for '%{O100}' + EXPECT_CALL(r, get_x(_)).WillOnce(Return(1)); + EXPECT_CALL(r, get_x(_)).WillOnce(Return(101)); + // Called for '%{O-100}' + EXPECT_CALL(r, get_x(_)).WillOnce(Return(101)); + EXPECT_CALL(r, get_x(_)).WillOnce(Return(1)); + // Called for '%{A1:cmd:}' + EXPECT_CALL(r, get_x(_)).WillOnce(Return(1)); + EXPECT_CALL(r, get_x(_)).WillOnce(Return(1)); + EXPECT_CALL(r, get_x(_)).WillOnce(Return(1)); + // Called for 'bar' + EXPECT_CALL(r, get_x(_)).WillOnce(Return(1)); + EXPECT_CALL(r, get_x(_)).WillOnce(Return(4)); + // Called for '%{O-3}' + EXPECT_CALL(r, get_x(_)).WillOnce(Return(4)); + EXPECT_CALL(r, get_x(_)).WillOnce(Return(1)); + // Called for '%{A}' + EXPECT_CALL(r, get_x(_)).WillOnce(Return(1)); + EXPECT_CALL(r, get_x(_)).WillOnce(Return(1)); + EXPECT_CALL(r, get_x(_)).WillOnce(Return(1)); + } + + bar_settings settings; + m_dispatch->parse(settings, r, "%{l}%{O100 O-100}a%{A1:cmd:}bar%{O-3}%{A}"); + + const auto& actions = m_action_ctxt->get_blocks(); + + ASSERT_EQ(1, actions.size()); + + const auto& blk = actions[0]; + + EXPECT_EQ(1, blk.start_x); + EXPECT_EQ(4, blk.end_x); + EXPECT_EQ(alignment::LEFT, blk.align); + EXPECT_EQ(mousebtn::LEFT, blk.button); + EXPECT_EQ("cmd", blk.cmd); +}