Re: [hackers] [slock] Fix my previous commit and some light refactoring

2016-09-02 Thread Quentin Rameau
> Heyho Quentin,
Yo Markus,

> After some reconsidering, I merged the patch as you proposed it. The
> other three are merged as well. Thanks for the contribution.
Thanks!



Re: [hackers] [slock] Fix my previous commit and some light refactoring

2016-09-02 Thread Markus Teich
Heyho Quentin,

Quentin Rameau wrote:
> What I mean is if it makes sense to have a timeout in one case, it's
> valable for all other cases too.
> Also that's a (maximum) timeout, not a strict delay. So when nothing
> gets in the way of grabbing input, slock is automatically started
> anyway without any waiting.

After some reconsidering, I merged the patch as you proposed it. The other three
are merged as well. Thanks for the contribution.

--Markus



Re: [hackers] [slock] Fix my previous commit and some light refactoring

2016-09-01 Thread Quentin Rameau
> I don't think a config.h variable is the best option. When starting
> slock from the terminal the problem does not occur, so you don't want
> a timeout in this case.
I don't really understand what you mean by that. Why wouldn't you want
a timeout then? Even when started from a terminal, the mouse can still
be grabbed for example.
What I mean is if it makes sense to have a timeout in one case, it's
valable for all other cases too.
Also that's a (maximum) timeout, not a strict delay. So when nothing
gets in the way of grabbing input, slock is automatically started
anyway without any waiting.

> For key bindings it's easy to just pass a
> command line flag. Therefore I favor a shell wrapper or the direct
> command line flag, while I think the direct flag approach is best,
> since it's job (waiting) can be done from C as easily as it would be
> in a shell wrapper, but it avoid one extra layer of complexity.
I used there the more or less arbitrary value of 600ms because the
default key auto-repeat delay of Xorg is 660ms.
The pro then for a command line option would be to be able to specify a
different timeout for each usage of slock (are there different ones?).

Making a behaviour with timeout and one without would add unuseful
(imho) complexity to slock code.
So I'd be to stick to timeout or no timeout at all, but not both.

Are there any other opinions beside those?



Re: [hackers] [slock] Fix my previous commit and some light refactoring

2016-09-01 Thread FRIGN
On Thu, 1 Sep 2016 15:05:09 +0200
Markus Teich  wrote:

Hey Markus,

> I don't think a config.h variable is the best option. When starting
> slock from the terminal the problem does not occur, so you don't want
> a timeout in this case. For key bindings it's easy to just pass a
> command line flag. Therefore I favor a shell wrapper or the direct
> command line flag, while I think the direct flag approach is best,
> since it's job (waiting) can be done from C as easily as it would be
> in a shell wrapper, but it avoid one extra layer of complexity.

we are discussing an edge-case here. Retrying for 600ms is after a lot
of consideration definitely the best way.
I also checked the possibility of force ungrabbing the grab by other
applications using XF86Ungrab, but it doesn't work.
If it had worked, a possibility would've been to wait indefinitely
after forcefully ungrabbing input; so we just wait for the user to
release his keys (if it takes 2 seconds or whatever, say Mr. Plinkett
uses his Linux box :P).

Cheers

FRIGN

-- 
FRIGN 



Re: [hackers] [slock] Fix my previous commit and some light refactoring

2016-09-01 Thread Markus Teich
Quentin Rameau wrote:
> My preference would have been to completely remove the waiting behaviour
> outside of slock and maybe provide a simple shell script wrapper like sleep 1;
> slock.  Another solution (as discussed with FRIGN) would have been to force
> other applications to ungrab input before trying to grab it, but I don't
> really like this intrusive behaviour.  I didn't think about your proposition,
> maybe that's good.  But I'd see it more as a config.h parameter than a command
> flag parameter.

Heyho Quentin,

I don't think a config.h variable is the best option. When starting slock from
the terminal the problem does not occur, so you don't want a timeout in this
case. For key bindings it's easy to just pass a command line flag. Therefore I
favor a shell wrapper or the direct command line flag, while I think the direct
flag approach is best, since it's job (waiting) can be done from C as easily as
it would be in a shell wrapper, but it avoid one extra layer of complexity.

--Markus



Re: [hackers] [slock] Fix my previous commit and some light refactoring

2016-09-01 Thread Quentin Rameau
> Heyho Quentin,
Hi Markus,

> in the commit message you mentioned spawning slock from other GUI
> applications via keybindings as the case where it won't work. If this
> is the only case, I think it would be cleaner to add a command line
> switch where the user can specify a wait time and after this timeout
> it is only tried once to grab mouse and keyboard.
Well, I mentioned this one as an example (dwm does that), but there may
be other use cases we don't think about yet.
I did it that way to restore the original behaviour, fixing my hurried
code modifications.
My preference would have been to completely remove the waiting
behaviour outside of slock and maybe provide a simple shell script
wrapper like sleep 1; slock.
Another solution (as discussed with FRIGN) would have been to force
other applications to ungrab input before trying to grab it, but I
don't really like this intrusive behaviour.
I didn't think about your proposition, maybe that's good.
But I'd see it more as a config.h parameter than a command flag
parameter.

What do you think?

> 
> --Markus
> 




Re: [hackers] [slock] Fix my previous commit and some light refactoring

2016-09-01 Thread Markus Teich
Quentin Rameau wrote:
> My last commit removing the input grabbing waiting loop was a mistake, I
> tested it on a specific setup which didn't raise any error while regular
> setups should do.  So the second of the four next commits fixes that.

Heyho Quentin,

in the commit message you mentioned spawning slock from other GUI applications
via keybindings as the case where it won't work. If this is the only case, I
think it would be cleaner to add a command line switch where the user can
specify a wait time and after this timeout it is only tried once to grab mouse
and keyboard.

--Markus



Re: [hackers] [slock] Fix my previous commit and some light refactoring

2016-09-01 Thread FRIGN
On Thu, 1 Sep 2016 13:45:24 +0200
Quentin Rameau  wrote:

Hey Quentin,

> My last commit removing the input grabbing waiting loop was a mistake,
> I tested it on a specific setup which didn't raise any error while
> regular setups should do.
> So the second of the four next commits fixes that.
> The other three try to move duplicated behaviour inside unique
> functions and move global variables inside functions which
> exclusively need them. Patches following.

the patches look good to me and I would favor them to be merged.
Thanks for your work, Quentin!

Cheers

FRIGN

-- 
FRIGN