This should have been addressed before merge, this is my failing. Sorry I
didn't review your branch closely enough.
On Mon, Aug 3, 2009 at 5:32 PM, Chris S. <email address hidden> wrote:
> This might require a bit more thought than simply removing that OrderBy.
> The old keybinding code did not organize this list logically based on
> function, it simply displayed each of the keybindings in the dictionary.
> With the old code, ALL keybindings were created in just one place, so as
> long as the "Summon" keybinding was registered first programmatically,
> "Summon" would show up first on that list.
This is how it should still be. Why would we be doing keybinding any place
else besides controller? We absolutely should not be.
> With the new keybinding code, we can register keybindings from anywhere.
> That being the case, even if we progamatically register "Summon" first
> (as your new branch up for review does), it still wouldn't be guaranteed
> to be the first one on that list, because code somewhere else might beat
> Do.Controller when registering a keybinding.
Again, this is wrong. We shouldn't be doing keybindings in any other place
besides controller init.
Either way, I agree that globally bound keys should show up at the top
> of the list, but I still like everything being sorted alphabetically.
Definitely not. The impetus for this bug was not that summon wasn't at the
top, but that first/last, previous/next pairs were not next to each other.
Alphabetizing is the cause of this.
This should have been addressed before merge, this is my failing. Sorry I
didn't review your branch closely enough.
On Mon, Aug 3, 2009 at 5:32 PM, Chris S. <email address hidden> wrote:
> This might require a bit more thought than simply removing that OrderBy.
> The old keybinding code did not organize this list logically based on
> function, it simply displayed each of the keybindings in the dictionary.
> With the old code, ALL keybindings were created in just one place, so as
> long as the "Summon" keybinding was registered first programmatically,
> "Summon" would show up first on that list.
This is how it should still be. Why would we be doing keybinding any place
else besides controller? We absolutely should not be.
> With the new keybinding code, we can register keybindings from anywhere.
> That being the case, even if we progamatically register "Summon" first
> (as your new branch up for review does), it still wouldn't be guaranteed
> to be the first one on that list, because code somewhere else might beat
> Do.Controller when registering a keybinding.
Again, this is wrong. We shouldn't be doing keybindings in any other place
besides controller init.
Either way, I agree that globally bound keys should show up at the top
> of the list, but I still like everything being sorted alphabetically.
Definitely not. The impetus for this bug was not that summon wasn't at the
top, but that first/last, previous/next pairs were not next to each other.
Alphabetizing is the cause of this.
--
-- Alex Launi