[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: UPDATE: command: shift_edge



Hey Guys,

First off, sorry for leaving such a fatal bug in my patch.
Good work though, Thorsten, for the quick fix. Shift/Focus commands aren't on my todo list anymore, but this might serve as a good reminder for all to check for these settings in the future (if focus_follows_shift isn't depreciated).

On Thu, Dec 27, 2012 at 08:53:33PM +0100, Thorsten Wi?mann wrote:
> So the open question is: What's the cleanest and best fixup?
> 
>   - Solution A: manually shift the window to the edge instead of
>     calling the shift command. This also would cause that locking and
>     unlocking the monitors isn't needed anymore in shift_edge.
> 
>   - Solution B: Remove the setting focus_follows_shift. Is anyone
>     out there using this option with the value 0? This option was
>     initially added for compatibility because the initial shift-command
>     didn't reset the focus.
> 
>     Please inform me via irc or the ML if you are using
>     focus_follows_shift = 0.
> 
>   - Solution C: Do A and B.
> 
> Discuss!

I don't personally have focus_follows_shift=0, but if any users are using it, I don't think it would hurt to keep the setting if devs and contributors are aware of it when writing involves shift commands.

====

* If focus_follows_shift IS kept, we must discuss the setting=0's effect when using shift_edge.

When focus_follows_shift=0, and shift command is used, would it be true that (effectively): the INDEX within the frame of the focused window before shifting, is equal to the index of the focused window after shifting? (Minus 1, of course, if the window is shifted out of the frame in the down or right directions?)
If so, this should also be the behavior of shift_edge, eg. the focused window index AND frame should be the same before and after an edge shift.


    1: A way to produce this behavior would be to increment an integer n each time shift_command() returns success, then call focus_edge in the opposite direction n times (or n+/-1, I'm having trouble visualizing it).
    This would result in the least amount of code, I think, but wouldn't be most efficient.

    2: An alternative option would to be to calculate the frame/window that would receive focus if focus_follows_shift=0 BEFORE performing the shift() loop, perform the loop, the set focus back to the frame/window.
    This would result in more code, but would be more efficient.

    3: We could also just calculate the number of windows/frame on the current tag, and call the shift_command in a for loop, instead of a while loop.
    This isn't very good IMO, but it'd fix the infinte loop problem (but it'd have undefined behavior when the user DOES do the steps that would reproduce the bug).


        Note that 1&2 would would require an equivalent to the quickfix: temporarily setting the GLOBAL focus_follows_shift=1.
        This is sloppy, IMO.
        A clean way to do 1&2 without this is to separate the shifting routine and the frame_move_window_command, and pass the new standalone shift function the focus_follows_shift setting; frame_move_window_command could pass the global, while edge_shift always passes 1.

        Note also though that doing 1|2 would also allow the rewriting of the FRAME STRUCTS and the call to monitor_apply_layout to be separate, eg. the restructuring of the frames would be done in the new shift function, and the call to monitor_apply layout would be done in frame_move_window_command and shift_edge_command.
        This would solve the locking/unlocking monitors thing, so we'd effectively be doing Thorsten's option A, minus the rewrite.
        Seperation would also only require ~5 extra lines of code.


    4: (About Thorsten's suggestion A) note that if we did separate frame_move_window from the shift routine, the routine would be NEARLY the same as rewriting shift_edge: we would still have to ascend the tree to get to the edge-most frame.
    The only difference is we would only have to descend once if we rewrote it (at the last frame) versus descending at each frame.
    This would result in more code, but would be MOST efficient.
    But its also not modular and therefore (among other aesthetic reasons), IMO, elegant.

    5: We could, of course, ignore focus_follows_shift for just shift_edge command.
    This is equivelent to the quickfix Thorsten implemented.
    Not very nice to the user IMO, but always an option.

My vote would be method #1 + the seperation.
Method #1 because I think it would rarely result in magnitudes more clock cycles than #2, its less code, and it's a rare situation anyway (really, who is using this setting?).
I'm not much for #4 if you couldn't tell :), but it is of course, a very valid option, if the weighted majority picks it.

======

* If the setting focus_follows_shift ISNT kept, we just have to decide between three things:
    whether we should separate the shift routine from the frame_move_window_command (as per note about #1&#2),
    continue calling frame_move_window_command with fake args,
    or rewrite focus_edge.

My vote would be the separation, as per my reasons about #4 above, and because it would make locking/unlocking the monitors unnecessary (as per my note about #1|#2.

=====

Let me know what you guys think
Tylo