[MPlayer-cvslog] r34546 - in trunk: DOCS/xml/de/skin.xml DOCS/xml/en/skin.xml gui/app.c gui/app

Ingo Brückl ib at wupperonline.de
Fri Jan 13 17:57:57 CET 2012


Reimar Döffinger wrote on Thu, 12 Jan 2012 20:20:18 +0100:

> On Thu, Jan 12, 2012 at 03:21:56PM +0100, ib wrote:
>> -        if (appFindItem(msg))
>> +        if ((msg == evMenu) || appFindItem(msg))

> Nit: At least I don't like extra () around == much

Normally I don't like it either (and don't do it), but in case an arithmetic
expression meets a boolean one, I find it more readable.

  if (this || that)
  if (this || that == 100)     // could be this || that until you see the ==
  if (this || (that == 100))

For the same reason, I prefer the extra () in:

  flags |= (enable ? MF_ENABLED : MF_GRAYED)

It's just habit.

>> +   case evMenu:
>> +        /*if (guiApp.menuIsPresent)   NOTE TO MYSELF: Not before mouse over and
>> +         {                                            cursor keys will work there.

> At least I can't parse that comment, I think the grammar is wrong.

I'll try again.

Ingo


More information about the MPlayer-cvslog mailing list