Skip to content
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

Add XY movement & refactor monitor types #18

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 12 additions & 12 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

21 changes: 21 additions & 0 deletions src/cli/modify/actions/displace.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
use gnome_randr::display_config::{
physical_monitor::PhysicalMonitor, ApplyConfig, monitor_models::transform::Displacement,
};

use super::{Action};

pub struct DisplacementAction {
pub displacement: Displacement
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like what you've done here with the string parsing, but there are a couple issues with it:

  1. Like I said below, I'd rather keep the scale as a separate argument and make this just the x,y
  2. The string parsing is now in the display_config directory, which is supposed to be a separate library. I'd rather avoid putting CLI details in the library wherever possible, which is why I had the rotation struct to begin with. This is a relatively minor transgression though so I don't mind much.

I would make a position struct that gets parsed to an x and y value and use that as the property on this Action.

}

impl Action<'_> for DisplacementAction {
fn apply(&self, config: &mut ApplyConfig, _: &PhysicalMonitor) {
config.transform.displacement = self.displacement;
}
}

impl std::fmt::Display for DisplacementAction {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
write!(f, "setting displacement to {}", self.displacement)
}
}
4 changes: 3 additions & 1 deletion src/cli/modify/actions/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,15 @@ mod mode;
mod primary;
mod rotation;
mod scale;
mod displace;

use gnome_randr::display_config::{physical_monitor::PhysicalMonitor, ApplyConfig};

pub use mode::ModeAction;
pub use primary::PrimaryAction;
pub use rotation::RotationAction;
pub use rotation::OrientationAction;
pub use scale::ScaleAction;
pub use displace::DisplacementAction;

pub trait Action<'a>: std::fmt::Display {
fn apply(&self, config: &mut ApplyConfig<'a>, physical_monitor: &PhysicalMonitor);
Expand Down
2 changes: 1 addition & 1 deletion src/cli/modify/actions/mode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ impl<'a> Action<'a> for ModeAction<'a> {
config
.monitors
.iter_mut()
.find(|monitor| monitor.connector == physical_monitor.connector)
.find(|monitor| monitor.connector == physical_monitor.monitor_description.connector)
.unwrap()
.mode_id = self.mode;
}
Expand Down
22 changes: 8 additions & 14 deletions src/cli/modify/actions/rotation.rs
Original file line number Diff line number Diff line change
@@ -1,27 +1,21 @@
use gnome_randr::display_config::{
logical_monitor::Transform, physical_monitor::PhysicalMonitor, ApplyConfig,
physical_monitor::PhysicalMonitor, ApplyConfig, monitor_models::transform::Orientation,
};

use super::{super::Rotation, Action};
use super::{Action};

pub struct RotationAction {
pub rotation: Rotation,
pub struct OrientationAction {
pub orientation: Orientation
}

impl Action<'_> for RotationAction {
impl Action<'_> for OrientationAction {
fn apply(&self, config: &mut ApplyConfig, _: &PhysicalMonitor) {
config.transform = match self.rotation {
Rotation::Normal => Transform::NORMAL,
Rotation::Left => Transform::R270,
Rotation::Right => Transform::R90,
Rotation::Inverted => Transform::R180,
}
.bits();
config.transform.orientation = self.orientation;
}
}

impl std::fmt::Display for RotationAction {
impl std::fmt::Display for OrientationAction {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
write!(f, "setting rotation to {}", self.rotation)
write!(f, "setting rotation to {}", self.orientation)
}
}
2 changes: 1 addition & 1 deletion src/cli/modify/actions/scale.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ pub struct ScaleAction {

impl Action<'_> for ScaleAction {
fn apply(&self, config: &mut ApplyConfig, _: &PhysicalMonitor) {
config.scale = self.scale;
config.transform.displacement.scale = self.scale;
}
}

Expand Down
70 changes: 24 additions & 46 deletions src/cli/modify/mod.rs
Original file line number Diff line number Diff line change
@@ -1,56 +1,20 @@
mod actions;

use gnome_randr::{display_config::ApplyConfig, DisplayConfig};
use gnome_randr::{display_config::{ApplyConfig, monitor_models::transform::{Orientation, Displacement}}, DisplayConfig};
use structopt::StructOpt;

use self::actions::{Action, ModeAction, PrimaryAction, RotationAction, ScaleAction};
use self::actions::{Action, ModeAction, PrimaryAction, OrientationAction, ScaleAction, DisplacementAction};

#[derive(Clone, Copy)]
pub enum Rotation {
Normal,
Left,
Right,
Inverted,
}

impl std::str::FromStr for Rotation {
type Err = std::fmt::Error;

fn from_str(s: &str) -> Result<Self, Self::Err> {
match s.to_lowercase().as_str() {
"normal" => Ok(Rotation::Normal),
"left" => Ok(Rotation::Left),
"right" => Ok(Rotation::Right),
"inverted" => Ok(Rotation::Inverted),
_ => Err(std::fmt::Error),
}
}
}

impl std::fmt::Display for Rotation {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
write!(
f,
"{}",
match self {
Rotation::Normal => "normal",
Rotation::Left => "left",
Rotation::Right => "right",
Rotation::Inverted => "inverted",
}
)
}
}

#[derive(StructOpt)]
pub struct ActionOptions {
#[structopt(
short,
long = "rotate",
help = "One of 'normal', 'left', 'right' or 'inverted'",
long_help = "One of 'normal', 'left', 'right' or 'inverted'. This causes the output contents to be rotated in the specified direction. 'right' specifies a clockwise rotation of the picture and 'left' specifies a counter-clockwise rotation."
long,
help = "A desired orientation of the display.",
long_help = "Any of ('normal', 'left', 'right' or 'inverted'). and optionally 'flipped' joined by ','. EG 'normal,flipped'. This causes the output contents to be rotated in the specified direction. 'right' specifies a 1/4 clockwise rotation of the picture and 'left' specifies a 3/4 clockwise rotation."
)]
pub rotation: Option<Rotation>,
pub rotation: Option<Orientation>,

#[structopt(
short,
Expand All @@ -65,6 +29,14 @@ pub struct ActionOptions {

#[structopt(long, help = "Set the scale")]
pub scale: Option<f64>,

#[structopt(
short,
long,
help = "A desired x,y, and scale.",
long_help = "A comma-separated list of displacement information '100,88,1'. The order is always X, Y, Scale. This does not check whether monitors can be arranged in that way."
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like that there's a repetition between --scale and --displacement, and I also am not a fan of comma-separated syntax and would like to keep it to a minimum.

Can you make this --position and allow position and scale to be set separately? Otherwise the behavior if you set both --displacement and --scale is unclear, and you end up with a more opaque --displacement 100,88,1 instead of --position 100,88 --scale 1 which reads a lot clearer to me.

)]
pub displacement: Option<Displacement>
}

#[derive(StructOpt)]
Expand Down Expand Up @@ -120,8 +92,8 @@ pub fn handle(
let primary_is_changing = opts.actions.primary;

if let Some(rotation) = &opts.actions.rotation {
actions.push(Box::new(RotationAction {
rotation: *rotation,
actions.push(Box::new(OrientationAction {
orientation: *rotation,
}));
}

Expand All @@ -137,6 +109,12 @@ pub fn handle(
actions.push(Box::new(ScaleAction { scale: *scale }))
}

if let Some(displacement) = &opts.actions.displacement {
actions.push(Box::new(DisplacementAction {
displacement: *displacement,
}))
}

if actions.is_empty() {
println!("no changes made.");
return Ok(());
Expand All @@ -162,11 +140,11 @@ pub fn handle(
.monitors
.iter()
.filter_map(|monitor| {
if monitor.connector == opts.connector {
if monitor.monitor_description.connector == opts.connector {
return Some(apply_config.clone());
}

let (logical_monitor, _) = match config.search(&monitor.connector) {
let (logical_monitor, _) = match config.search(&monitor.monitor_description.connector) {
Some(monitors) => monitors,
None => return None,
};
Expand Down
Loading