Conversation
src/layout/floating.rs
Outdated
| let win_height = ensure_min_max_size(win_height, min_size.h, max_size.h); | ||
|
|
||
| let win_size = Size::from((win_width, win_height)); | ||
| if win_size.w > 0 |
There was a problem hiding this comment.
IMHO because of how many conditions are in this block, a comment would go a long way to aiding readability e.g.
// Skip redundant request when window already matches size
if win_size.w > 0
// ...In general, as a newcomer to this codebase, it's not clear to me why we would need to skip it. Whether or not it's worth a mention in the comment I would leave to you, it may be obvious to maintainers.
There was a problem hiding this comment.
Thank you for your review. I refactored that predicate into should_skip_redundant_size_request() and added a comment explaining the intent. The skip is only for the case where the floating window already has the requested concrete size and there isn’t another pending state change that still needs a configure; that avoids redundant same-size configures, but still allows the request to be re-sent after a stale commit.
src/layout/floating.rs
Outdated
| let win_width = ensure_min_max_size(win_width, min_size.w, max_size.w); | ||
|
|
||
| let win_size = Size::from((win_width, win_height)); | ||
| if win_size.w > 0 |
There was a problem hiding this comment.
As above, could do with a comment e.g.
// Skip redundant request when window already matches size
ChrisPWill
left a comment
ChrisPWill
left a comment
There was a problem hiding this comment.
LGTM, thanks! The intent is much clearer to me now
3 participants
Summary
request_size_once()case where a repeated same-size request could be dropped after a stale commit.Behavior Changes
tests::floating::repeated_size_request:tests::floating::resize_to_same_size:Validation
repeated_size_requestresize_to_same_sizeresize_to_different_then_sameresize_during_interactive_move_propagates_to_floating