Hi

On Tue, May 13, 2014 at 01:41:07PM -0700, Keith Amling wrote:
> The various things I didn't quote are all straight-forward and will be
> my pleasure to fix.
> 
> > I don't think it'd be that much compatibility code.
> > 
> > We could just maintain a flag for the "option-set" prefix in the root
> > table, and that can be used for prefix/send-prefix. The root table can
> > never be deleted so that isn't an issue.
> > 
> > prefix2 can go away once there is an alternative.
> > 
> > Still, if you want to make the other changes first I will look again and
> > we can decide on this later.
> 
> Sorry, I'm obviously falling behind here; I just don't follow what
> you're proposing with enough clarity to implement it.  Are you
> suggesting another KEYC_* flag (or two?) for "this is the binding set by
> `set-option prefix[2]`"?  Are you proposing an extra field (or two?) in
> key_bindings to hold the prefix in the root table?  I don't mean to be a
> drag and I'd rather do what I can to minimize your work explaining your
> vision to chumps and/or having to clean up our crappy patches but I'm
> just not following this one.

I was suggesting an extra flag in the struct key_binding to mark the
current "option-set" prefix in the root table. Basically when you change
the option, it'll find the one with the flag and move it.

It's just an idea - thinking again it may be much the same just to leave
prefix as you have it, or to do something similar.

So long as I can add new prefixes by binding keys in the root table to
"switchc -Tprefix", it may be much the same to check the prefix option
too (so it is an implicit, invisible binding to "switchc -Tprefix").

In any case leave it as it is for the next diff and I'll look at it
again then.

> 
> I'll clean the patch up for the other stuff first anyway.
> 
> > > > - I think it would be better to point the client to a struct
> > > >   key_bindings, not to a string with the name. I know it won't save the
> > > >   lookup but it's neater than doing xstrdup/free all over the
> > > >   place. When a table is deleted you can walk the clients and reset them
> > > >   (or alternatively, reference count).
> > > 
> > > I agree xstrdup/free all over the place is a mess but keeping a pointer
> > > to an actual table seems like an accident waiting to happen in terms of
> > > safety.
> > 
> > You could say this about any pointer.
> > 
> > > Additionally it provides weirdness if e.g.  a user sets a
> > > keytable, then unbinds the last key in the keytable and rebinds it
> > > (causing the client to point to a stale empty keytable).
> > 
> > I don't see that this will be able to happen - when the user unbinds the
> > last key the table needs to be destroyed, at that point the client
> > should be fixed up (or the table destroy deferred until the client is
> > finished with it).
> 
> Well, I agree it needs to be destroyed eventually but the issue is that
> if a user thinks of the client as being set to [read from] a keytable
> with a name then they will likely think binding a key into a keytable of
> that name will be the same table even if the bind happens after the set.

This will still work if the key table is a pointer, both the bind and
the set will use the same key table.

Or are you thinking of creating the table after running the set command?
This shouldn't work - you shouldn't be able to set a client to a
nonexistent table.

> 
> My opinion about safety is completely countered by ref counting (it was
> the final unbind's deletion having to know about and clean up
> outstanding references from clients that was worrying me) and the
> weirdness with stale keytables of duplicate names isn't that big of a
> deal for the sanity we're gonna save in code.

They don't need to know about the specific references from clients, they
can check every client in the clients list. All clients will be in the
list and nobody will have more than at most a few hundred so walking
them all would be fine.

Or reference counting the tables would work too if you prefer.

> 
> Keith

------------------------------------------------------------------------------
"Accelerate Dev Cycles with Automated Cross-Browser Testing - For FREE
Instantly run your Selenium tests across 300+ browser/OS combos.
Get unparalleled scalability from the best Selenium testing platform available
Simple to use. Nothing to install. Get started now for free."
http://p.sf.net/sfu/SauceLabs
_______________________________________________
tmux-users mailing list
tmux-users@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tmux-users

Reply via email to