-
-
Notifications
You must be signed in to change notification settings - Fork 86
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feature added, move-column-left-or-monitor-left and move-column-right… #528
Conversation
…-or-monitor-right
Shouldn't the action name be |
Will you also do |
prob not, it's not personally useful to me, and i don't even know if i implemented this right |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Almost good to go, just some grammatical renames and a condition fix.
niri-config/src/lib.rs
Outdated
MoveColumnLeftOrMonitorLeft, | ||
MoveColumnRightOrMonitorRight, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
MoveColumnLeftOrMonitorLeft, | |
MoveColumnRightOrMonitorRight, | |
MoveColumnLeftOrToMonitorLeft, | |
MoveColumnRightOrToMonitorRight, |
For grammar and consistency sake (like i.e. MoveWindowDownOrToWorkspaceDown
)
niri-ipc/src/lib.rs
Outdated
/// Move the focused column to the left or to the left monitor | ||
MoveColumnLeftOrMonitorLeft, | ||
/// Move the focused column to the right or to the right monitor | ||
MoveColumnRightOrMonitorRight, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/// Move the focused column to the left or to the left monitor | |
MoveColumnLeftOrMonitorLeft, | |
/// Move the focused column to the right or to the right monitor | |
MoveColumnRightOrMonitorRight, | |
/// Move the focused column to the left or to the monitor to the left. | |
MoveColumnLeftOrToMonitorLeft, | |
/// Move the focused column to the right or to the monitor to the right. | |
MoveColumnRightOrToMonitorRight, |
src/layout/mod.rs
Outdated
@@ -1196,6 +1196,36 @@ impl<W: LayoutElement> Layout<W> { | |||
monitor.move_column_to_last(); | |||
} | |||
|
|||
pub fn move_column_left_or_output(&mut self, output: &OutputId) -> bool { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pub fn move_column_left_or_output(&mut self, output: &OutputId) -> bool { | |
pub fn move_column_left_or_to_output(&mut self, output: &Output) -> bool { |
These should accept &Output
not &OutputId
.
src/layout/mod.rs
Outdated
let workspace = monitor.active_workspace(); | ||
let curr_idx = workspace.active_column_idx; | ||
|
||
if !workspace.columns.is_empty() && curr_idx != 0 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if !workspace.columns.is_empty() && curr_idx != 0 { | |
if !workspace.columns.is_empty() && curr_idx != workspace.columns.len() - 1 { |
src/layout/mod.rs
Outdated
MoveColumnLeftOrMonitorLeft(#[proptest(strategy = "1..=2u8")] u8), | ||
MoveColumnRightOrMonitorRight(#[proptest(strategy = "1..=2u8")] u8), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
MoveColumnLeftOrMonitorLeft(#[proptest(strategy = "1..=2u8")] u8), | |
MoveColumnRightOrMonitorRight(#[proptest(strategy = "1..=2u8")] u8), | |
MoveColumnLeftOrToMonitorLeft(#[proptest(strategy = "1..=2u8")] u8), | |
MoveColumnRightOrToMonitorRight(#[proptest(strategy = "1..=2u8")] u8), |
@YaLTeR, added your fixes, should be ready now |
Looks like you've changed the lines that I commented about but not the other lines that depend on them, or the same name used elsewhere |
oh yea, sorry |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good.
Thanks |
feature added, move-column-left-or-monitor-left and move-column-right-or-monitor-right
based on #456