[openbox] client list with submenus

Dana Jansens dana at orodu.net
Tue Jan 4 16:06:46 EST 2011


2010/12/19 Alexey Korop <akorop at gmail.com>:
> Alexey Korop wrote on 29.11.2010 22:52:
>>
>> Now I'm trying to change client items of the
>> client-combined-menu to the submenus with the default item "Switch to".
>
>     I'm ready. This innovation also applies to the client-list-menu.
>     The following text contains mostly technical details. User-level text
> with screenshots see here: http://fit.kharkiv.org/openbox/
>
>     This feature is based on the menus with default choice. I sent Bugzilla
> patch that implements it (http://bugzilla.icculus.org/show_bug.cgi?id=4795),
> but this patch was not commited, so I include it in modified form in a new
> patch attached to this letter. Attached patch should be applied not after,
> but instead of the patch of bug 4795.
>
>     Now I made the following.
>
>     1. Submenus with default are reimplemented.

Thanks for the patch.

>     1.1. Now default item is marked in a expanded submenu by the same symbol
> as a menu item mark, but placed at the right of the bar.

In the example the default menu item doesn't have an icon of its own.
If it did would the icon not show up, or would the default marker not
show up?  What if we bolded the default entry instead? Then it would
always be visible.

>     1.2. Now the default item may be marked in the menu.xml by new item
> property "default", f.e.  <item default="true" label="my-label">.

Cool, that's good.

>     1.3. Default item temporary may be marked by '*' as firsr character of a
> label. This '*' is not displayed. This possibility can be removed after
> Obmenu will support the menu with default.

I really don't want to do it this way.  This makes legacy behaviour
that you can never remove, because people will expect it and build
around it.  ObMenu should just be updated and until then it just
doesn't support this option.

>     1.3. New menu property "has_default" added. It may be used for
> pipe-menus, f.e. <menu has_default="true" execute="my_script" id="my_script"
> label="my pipe menu"/>. If the menu has this property, then its row in the
> parent menu will be marked with the symbol of a menu with default.

This is messy because it depends on the user to know details about the
pipe menu when adding it to their menus.  The pipe menu should dictate
this stuff itself.  Users don't/shouldn't have to read the menu and
understand if it has a default or not when adding it to their menu.

I realize it's a bit of a catch-22.  You want to mark the submenu
entries that have default-behaviour attached to them.  But you can't
run all the pipe menus, and their default-ness may even change from
time to time, so one execution may differ from the next.  has_default
could be marked true but no default will be found when you actually
run the menu, so the submenu would appear to be lying to you.

I want it to behave consistently.  If the pipe menu is going to
provide a default, it should have to do so upfront, and Openbox should
cache it while the menu is open, so if you click it, the menu is not
run again, and the default given cannot change/disappear suddenly.

1. The easiest would just be to not let a pipe menu have a default
action for its top level.

2. Another would be to add an extra expectation to pipe menus scripts
to provide for a default entry when asked for it.  For instance by
passing an environment variable to the script when running it.  This
would make legacy pipe menus slow down menus though since theyd run
fully, and not return a default anyways.

3. Yet another would be to add a little protocol.  For instance, when
the script is run it doesn't return anything until queried from stdin.
 Openbox could send "default" or "full" as commands, and the script
could return the one it asked for.  This is more extendable in the
future, but also breaks old scripts.

If it's an important feature we could do 3.  For the moment, I'd
rather stick with 1, as this would make a nice separate enhancement.
Let's just not let pipe menus declare a default at their top level,
and we can come back to it after.

>     2. Submenu row in a parent menu now may have an icon. This involved
> changing the definition of ObMenuEntry. The fields related to the icon are
> moved from data.normal to a common part.
>
>     3. The client-list-menu and the client-list-combined-menu now have 90%
> of the same user-level functions, so the splitting of its code to the two
> files lost all meaning. I remove the files "client-list-combined-menu.*" and
> move his functions to the "client-list-menu.*".
>
>     I've used as a base the last snapshot of the  Dana's "work" branch (15
> Dec 2010).
>     The proposed patch is quite big, but it brings a little new code. For
> the most part, it reorganizes the old code. The number of source lines is
> almost no difference.
>
>     I don't send this patch to the Bugzilla because I don't understand what
> I need to do with internationalization. There are two new
> internationalizable strings ("_Activate" and "_Kill") and several strings

The Close action already allows for killing when the application isn't
responding.  So I think that adding another option for killing
clutters the interface and can lead to a lot of lost work by
accidental clicks that bypass any save dialogs etc.  So I don't think
the kill option needs to be in the menu.

> have changed their location. The patch attached contains the differences for
> some language files, but I doubt that it is done correctly.  Four language
> files are patched: "po/openbox.pot", "po/POTFILES.in", "po/ru.po" and
> "po/uk.po". The essential is to correct only the file "POTFILES.in" of which
> removed the reference to a file "openbox/client_list_combined_menu.c" that
> no longer exists.

For translation changes to line numbers, we don't commit any change in
git for those, we only let the numbers update when building release
tarballs.  Generally code changes only include changes to the .pot and
POTFILES as you've done, and translation changes are done separately.
What you've done there looks good to me, unless Mikael has another
opinion on it.

Some style things with the patch.  Variable declarations should always
be at the top of the code block.  For example, at line 340 of the
diff.  They are usually followed by a blank line.

The ACTIONS_TAG and SEND_TO_TAG seem like hidden tricks that will hurt
to trip over in the future.  Why are you using those?  It seems like
you're making a new menu for each client in the menu, making it tricky
to tell if the client menu is showing.  Why not use a single menu for
all the clients, then attach it as a submenu for all of the clients.
When an action in the menu is executed, you can figure out which
client to execute on from looking at what was selected in the parent
menu.  This would also save on memory and make the menu much faster to
show.


Cheers,
Dana

> Merry Christmas!
>
> Yours truly Alexey
>
>
> _______________________________________________
> openbox mailing list
> openbox at icculus.org
> http://icculus.org/mailman/listinfo/openbox
>
>


More information about the openbox mailing list