Merge lp://qastaging/~lukas-vacek/unity/bamficon_windowlist-raring into lp://qastaging/unity

Seems like it could use a test.

10 + WindowManager& wm = WindowManager::Default();
11 + // delete all menu items for windows
…
29 + char menu_item_name[name_size];
30 + sprintf(menu_item_name,"w_list_%d",i);
…
44 + _menu_items[menu_item_name] = menu_item;

Please, don't add these items to _menu_items. Use instead another container (such as
std::vector<glib::Object<DbusmenuMenuitem>> _windows_menu_items and add the items you generate there in AddMenuItemsWindowList.

22 + for ( auto w: Windows() ) {

Use for (auto const& w : Windows())

At that point call it into GetMenus() putting the items you computed into the result list.

36 + _gsignals.Add(
37 + new glib::Signal<void, DbusmenuMenuitem*, int>(menu_item,

You can use the utility Add function to save some space:
_gsignals.Add<void, DbusmenuMenuitem*, int>(menu_item, DBUSMENU_MENUITEM_SIGNAL_ITEM_ACTIVATED ...

39 + [w,&wm] (DbusmenuMenuitem*,int) -> void {

Just pass w, while generate the WM inside the lambda body (even if that pointer won't change, it's safer not to pass by reference inside a lambda that is going to be executed when the local variable is dead). Also it's not needed to specify the "-> void" return type. g++ will guess it for you.

45 + std::string winName( ( wm.GetWindowName( w->window_id() ) ) );
46 + dbusmenu_menuitem_property_set(menu_item,
47 + DBUSMENU_MENUITEM_PROP_LABEL, winName.c_str() );

Only use:
dbusmenu_menuitem_property_set(menu_item, DBUSMENU_MENUITEM_PROP_LABEL, w->title().c_str());

51 + if (windows_added) {
52 + // add seperator
…
65 + }

Do this into GetMenus.

Also, long quicklist items should be cut. I'll provide I way to do it as I need it also on another branch, please ping me on IRC in case you'd need help.

Fix the style to match the one we use in unity (2 spaces indentation, brackets on next line)

Need tests.

Also the maximum string length of each Window title should be limited to 45 characters including spaces. Anything more than 45 characters should be chopped at 43 characters and have "..." applied to the end e.g.

"bamficon_windowlist_rearing : Code : Unity..." (In the case of this window)

> Also the maximum string length of each Window title should be limited to 45
> characters including spaces. Anything more than 45 characters should be
> chopped at 43 characters and have "..." applied to the end e.g.
>
> "bamficon_windowlist_rearing : Code : Unity..." (In the case of this window)

Yes, I agree with this (as I wrote above).
The fix for that is needed also by bug #893652 and will be provided by lp:~3v1n0/unity/quicklist-max-label-width.

Just set the QuicklistMenuItem::MAXIMUM_LABEL_WIDTH_PROPERTY DBusMenuItem int property (wanted px-width will be provided by design).

Update to my comment above:

- The max width should be 300px
- The window title should be ellipsized at the end
- This entry should only be displayed if there are 2 or more windows. If the application only has 1 or 0 windows open nothing should be displayed.

The format of the option should be:

====================
[application specific quicklist options area]
--------------
[application name]
--------------
Windows:
[window title 1]
[window title 2]
[window title 3]
--------------
Most recent files:
file one.odt
file two.odt
file three.odt
file four.odt
file five.odt
--------------
Keep in launcher
Quit
====================

thanks!

36 + // delete all menu items for windows
37 + _menu_items_windows.clear();

Please do this on EnsureMenuItemsWindowsReady

17 + _gsignals.Add<void, DbusmenuMenuitem*, int>(menu_item, DBUSMENU_MENUITEM_SIGNAL_ITEM_ACTIVATED,
18 + [w] (DbusmenuMenuitem*,int) {

This is ok, but since using [w] means copying the the WindowPtr, I'd prefer if you instead do:

Window xid = w->window_id();
_gsignals.Add<void, DbusmenuMenuitem*, int>(menu_item, DBUSMENU_MENUITEM_SIGNAL_ITEM_ACTIVATED, [xid] (DbusmenuMenuitem*,int) {
...
wm.Raise(xid);
}

22 + } );

Remove this padding, please (use also for the lambda two spaces for indenting).

24 + dbusmenu_menuitem_property_set(menu_item, QuicklistMenuItem::MAXIMUM_LABEL_WIDTH_PROPERTY, "300");

That property is an integer, you should do instead:

dbusmenu_menuitem_property_set_int(menu_item, QuicklistMenuItem::MAXIMUM_LABEL_WIDTH_PROPERTY, MAXIMUM_QUICKLIST_WIDTH);

Where MAXIMUM_QUICKLIST_WIDTH is a const int that you should define in the unnamed namespace that we have at the top of the ApplicationLauncherIcon.cpp file ;).

40 + // add windows menu items
41 + if (_menu_items_windows.size() > 1 )

Mh, I'd prefer to iterate one more time than allocating some extra memory here, so just use if (Windows().size() > 1), and move EnsureMenuItemsWindowsReady under this check.

Thanks ;)

Thanks for your fast reply. I will fix that and push the changes tonight.

> 24 + dbusmenu_menuitem_property_set(menu_item,
> QuicklistMenuItem::MAXIMUM_LABEL_WIDTH_PROPERTY, "300");
>
> That property is an integer, you should do instead:
>
> dbusmenu_menuitem_property_set_int(menu_item,
> QuicklistMenuItem::MAXIMUM_LABEL_WIDTH_PROPERTY, MAXIMUM_QUICKLIST_WIDTH);
>
> Where MAXIMUM_QUICKLIST_WIDTH is a const int that you should define in the
> unnamed namespace that we have at the top of the ApplicationLauncherIcon.cpp
> file ;).
>

I am not sure about this one - to me it seems the property is const char* and it won't compile if I set it to an integer. I am not sure how to fix my code here.

> Thanks for your fast reply. I will fix that and push the changes tonight.
>
> > 24 + dbusmenu_menuitem_property_set(menu_item,
> > QuicklistMenuItem::MAXIMUM_LABEL_WIDTH_PROPERTY, "300");
> >
> > That property is an integer, you should do instead:
> >
> > dbusmenu_menuitem_property_set_int(menu_item,
> > QuicklistMenuItem::MAXIMUM_LABEL_WIDTH_PROPERTY, MAXIMUM_QUICKLIST_WIDTH);
> >
> > Where MAXIMUM_QUICKLIST_WIDTH is a const int that you should define in the
> > unnamed namespace that we have at the top of the ApplicationLauncherIcon.cpp
> > file ;).
> >
>
> I am not sure about this one - to me it seems the property is const char* and
> it won't compile if I set it to an integer. I am not sure how to fix my code
> here.

Keep in mind you need to use dbusmenu_menuitem_property_***set_int***

Ah, also... It would be needed an unit-test or AP test. If you need some guidance, ping me on IRC.

I think it's now ready to go in trunk... We have tests and correctly cut long entries!

New screenshot for design re-review: http://i.imgur.com/NUfgdKm.png

Looks good to me now, thanks!