Re: [RP] ratpoison, patches, and the future

[Joren Van Onder]

On Tue, Dec 30, 2014 at 12:15, Jeff Abrahamson wrote:
>> > I've come up with pushwindow, pullwindow and focus_policy. Does
>> > that sound right?
>>
> Yes, this is precisely correct. I am embarrassed that I forgot to provide a
> summary of what I'd done when I sent that email.

No problem, just making sure.

>> It's not clear to me whether push/pullwindow are just syntactic sugar or
>> not, as put by Johannes Altmanninger:
>>
>> http://lists.nongnu.org/archive/html/ratpoison-devel/2014-10/msg00000.html
>>
> A sufficiently curmudgeonly computer programmer might claim that everything
> beyond a Turing Machine is syntactic sugar. The more pertinent question to
> me is whether it provides a useful abstraction (Johannes, Joren, and I say
> yes; Jérémie weighs in against; hundreds more say nothing ;-) and does it
> detract from those who don't want it (no complaints of additional slowness
> in the command parser so far ;-).

I do have to admit that I, just like Johannes, wasn't aware of the
--interactive flag (thanks to Jérémie for linking that email). So
pushwindow is more or less equivalent to:

    ratpoison --interactive -c swap -c focuslast -c other

I tested it a little bit and it does seem to work the same as
pushwindow. Johannes mentions that it does behave oddly when pushing a
window to the same frame, but in my opinion that's not really a huge
deal because there is no real reason to ever do that (except maybe
accidentally). Johannes also mentions performance as a potential issue
with this approach. Although a native implementation will always be
faster than sending three separate commands, I don't think the (tiny,
certainly not noticeable on my machine) performance difference is really
an issue here. If someone does notice a performance difference then I
believe the way ratpoison interprets commands should be investigated to
resolve this.

That being said, I'm not necessarily opposed to the push/pullwindow
commands. In the end it does come down to whether or not it is a useful
abstraction to give to the user in a default ratpoison environment. It
also depends on the quantity and quality of code necessary to implement
this feature. I leave the decision to Jérémie. In the end, he is the
one who needs to be willing to maintain the code in case Jeff disappears
for some reason (nothing personal, it's just something that is quite
common in FOSS, someone writes a bunch of patches and two years down the
line they cannot be reached anymore). Either way, I will probably keep
using the feature, either with the native push/pullwindow commands or
by aliasing the three separate commands.

> Thanks for the positive feedback. I want to set up some tests and some
> performance measures first, as (1) I'd like some assurance that my breakage
> surface is small, and (2) I'd like to be able to brag about how much faster
> it is. ;-)

Sure, measuring what impact your changes have is a great way to do things.

> Jérémie sent me some code feedback in private. Most of it was a bit
> ordinary stuff that needn't be repeated. Worth repeating here,
> however:

Thank you for putting these on the mailing list.

> - He expressed a concern that I developed all of these on the same
> branch. Actually, I did (and do) all of my work on separate branches, but
> in frustration that the code wasn't being merged into HEAD, I did it myself
> in my repository to make it easier to use the code, both for me and for
> others to try.

I understand what you were trying to do, but it's not ideal for
merging. Because right now it's either merge all or nothing (well, those
are the simplest options at least, you could always do more complicated
things to merge only parts). Also, the separate feature branches are
nice for people looking at the code because then all the changes are
logically grouped per feature. In order to keep it easy for people to
try out your version of ratpoison (with all the new stuff) you can still
rebase everything to upstream ratpoison (separate branch), and have your
master contain all the feature branches. This way you keep the
individual feature branches for inspection and merging with
upstream. Atlassian has a good explanation on how to maintain nice clean
feature branches [1].

> - He proposes that focus_policy become focus or focuspolicy (I prefer
> the latter, other opinions?). He also proposes that the third option be
> follows rather than ffm, I have no objection.

I also prefer focuspolicy, it is more descriptive and focus already
exists in ratpoison. It's a command not a variable, so it could
theoretically work but I feel like that might be confusing. Follow is a
nice improvement I think. Abbreviations tend to be confusing because
most people won't know that it stands for focus follows mouse. It might
also be nice to add a short explanation to the man page of the
difference between sloppy and follows. I know you've documented it in
the TeX, but I feel like like the vast majority of users won't even know
that that exists.

> - He's not so keen on my refactoring of cmd_select() and
> set_active_window_body(), suggesting they don't bring real improvement. I
> disagree. Both functions were overly long to my eye and harder to
> understand for it. (A quick git annotate on window.c even suggests that
> Jérémie may be the author of the FIXME on set_active_window_body(). ;-)

I'm not familiar enough with the code to really say anything about this
but I did take a quick look at the set_active_window_body change you
made (ca95ed9c). I'm not sure that extracting a function like you did
really fixes up the function as the FIXME intended. We could discuss the
benefits of this change and whether or not it improves readibility in a
significant way in depth but I feel like your other work/plans are much
more interesting and exciting things to talk about.


Footnotes:
[1] 
https://www.atlassian.com/git/tutorials/comparing-workflows/feature-branch-workflow

Happy holidays,
--
        Joren

signature.asc
Description: PGP signature