[MPlayer-dev-eng] [PATCH] Remove osd menu hardcoded keybindings, support custom keybindings.

[Ulion]

2007/12/2, Alban Bedel <albeu at free.fr>:
> On Sun, 2 Dec 2007 19:54:57 +0800
> Ulion <ulion2002 at gmail.com> wrote:
>
> > 2007/12/2, Reimar Döffinger <Reimar.Doeffinger at stud.uni-karlsruhe.de>:
> > > Hello,
> > > On Sun, Dec 02, 2007 at 08:50:09AM +0100, Alban Bedel wrote:
> > > > On Sat, 1 Dec 2007 14:18:11 +0800
> > > > Ulion <ulion2002 at gmail.com> wrote:
> > > >
> > > > > Hello,
> > > > >
> > > > > There are keybindings in libmenu for browse osd menu, but it's
> > > > > hardcoded, can not be customized.
> > > > > This patch remove the hardcoded keybindings, and support user
> > > > > defined keybindings from osd menu  browsering.
> > > > > As we known, each input key has only one possible binding
> > > > > defined input.conf, such as arrow up, down, left, and right,
> > > > > but when browsing osd menu, you probably want to use these
> > > > > keys, then they are hardcoded in libmenu. But for users want to
> > > > > use other key to browse osd menu, there's no way to custom
> > > > > browsing keybindings, so I made this patch fix the problem.
> > > >
> > > > Good idea, the patch seems fine. But why do you change the
> > > > read_key() return type from void to int? afaict from the patch
> > > > it's useless, but as it's mixed changes that's hard to say. If
> > > > you *realy* need to change the prototype then do it in another
> > > > patch.
> >
> > Change read_key() to int is for the next step: left un-handled keys to
> > input instead of steal all of them.  Currently it can be void, I will
> > remove it for a clean patch.
>
> You mean having keys not handled by the menu be handled as if there was
> no menu? I don't think that's a good idea. The normal key binding is
> too crazy to have that as a fallback.
>
> > >
> > > The same question applies as to why the menu_def_t typedef is
> > > removed. Makes me think if maybe the patch is a bi too big for what
> > > it tries to do and at least should be split.
> >
> > That's needed by keybindings' code, current menu struct has no type
> > info, the typedef is moved to the header file to add the type info
> > into menu struct so we can get menu's type name and menu's name.
> > But I can split this little part as a standlone patch and apply it
> > first to simply the main patch code.
> >
> > Here's the patch after cleanup. Other change: change menu cmd name
> > from macro name to lower case such like 'menu up', 'menu down'.
>
> I have been thinking about this a bit more. It can be made in a simpler
> way. On a keypress, when there is a bind it should just be queued as a
> normal mplayer command. If no bind is found the keypress is passed
> to the menu.

It could, although it's an indirect way to send menu cmd, but it looks
more extendable, new patch queue cmd is here.

>
> +<keybindings name="cmdlist" parent="list" />
>
> Hmm, 'name' is not good here, it should be 'menu-type' or something
> like this, that's what it is.

It not only match menu's type, indeed it first try to match menu's
name, so each menu can have different key bindings even with same menu
type. If menu name match fails, then try to match menu type name, if
still fails, fallback to use default key bindings.
So 'name' is the only word by now I can thought to use, and we can
consider it as the keybindings' name. Or some other words you can
suggest except 'menu-type'?


-- 
Ulion
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: osdmenu_custom_keybindings3.diff
URL: <http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/attachments/20071203/bae0257d/attachment.asc>