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 options for relative positioning of window on screen #21

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Add options for relative positioning of window on screen #21

wants to merge 2 commits into from

Conversation

udiboy1209
Copy link

Added window-position-x and window-position-y as double value 0.0-1.0 to allow user to move the window on the screen.

Added 'window-position-x' and 'window-position-y' as
double value 0.0-1.0 to allow user to move
the window on the screen.
@prikhi
Copy link
Owner

prikhi commented Mar 28, 2018

Cool, thanks! Will test it out soon.

Have you tested to see if it properly handles configs that don't have the new options?

@udiboy1209
Copy link
Author

I have not tested without adding those options. How is that handled, can you show me? I will add it to the code.

Copy link
Owner

@prikhi prikhi left a comment

Choose a reason for hiding this comment

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

get_double says it returns 0.0 & sets the GError when it can't find the key, so you need to pass a GError instead of null & check that error after calling the function. If it's not found or not a double, set it to 0.5.

src/config.c Outdated
position_y = 0.5;
}
config->position_x = position_x;
config->position_y = position_y;
Copy link
Owner

Choose a reason for hiding this comment

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

I think you should turn this into a function so you can just do something like:

config->position_x = get_relative_position(keyfile, "x");
config->position_y = get_relative_position(keyfile, "y");

@udiboy1209
Copy link
Author

@prikhi I have checked with a missing "window-position-x" conf and the latest commit works.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants