Hi,
[ I have no time at all to really give this the attention it deserves, so
I'm going to have to be brief and merely point you in the right directions.
I will though expect this to go through several iterations as a result. ]
On Sun, Oct 30, 2011 at 11:54:29PM +0100, Thomas Funk wrote:
> >-----Ursprüngliche Nachricht-----
> >Von: "Thomas Adam" <tho...@fvwm.org>
> >Gesendet: 20.10.2011 15:00:58
> >An: "Thomas Funk" <t.f...@web.de>
> >Betreff: Re: Reworked fvwm-menu-desktop
> >
> >>Some very quick observations...
> [...]
>
> >>Can you describe the internal data overall, and now it's derived, and then
> >>we can look at how best to process it. That way, we can reduce a lot of the
> >>clutter, and define a more cleaner interface for all of this.
>
> Hi Thomas,
> sorry for the delay, but I was seriously ill the last days. But now I've done
> the description of merge_fvwm2_menus.
It's fine. I wasn't interested in the descriptions actually, since I can
read your code fine. I wanted the internal structure which you've now
given.
> sub merge_fvwm2_menus {
> my $weight = 0;
> my $reference_menu;
> # check first if xdg_menu_prefix is set
> if (defined $xdg_menu_prefix) {
> $reference_menu = $xdg_menu_prefix;
> } else {
> # check if applications.menu exist
> # question: correct to disable autovivification?
> if (ref $fvwm2_menus{''} eq 'HASH') {
> $reference_menu = '';
This is ultimately incorrect, but I think you now understand why.
> } else {
> # get the biggest menu as reference
> foreach my $prefix (keys %fvwm2_menus) {
> my $size = scalar keys %{$fvwm2_menus{$prefix}};
Can just say:
my $size = (keys %{...});
> ## compare the other menus with reference and delete same entries
> # create an fvwm menu hash
> %{$fvwm2_menus{'fvwm-'}} = %{$fvwm2_menus{$reference_menu}};
> # get the first hash key ('gnome-', 'kde-', 'fvwm-')
> foreach my $compare_menu (keys %fvwm2_menus) {
> # don't check the reference or the fvwm menu
> next if ($compare_menu eq $reference_menu || $compare_menu eq
> 'fvwm-');
>
> # get the menu names from the reference menu (check the known menus
> for same entries)
> foreach my $refkey (keys %{$fvwm2_menus{$reference_menu}}) {
> # get the menu names from the menu should compared
> foreach my $compkey (keys %{$fvwm2_menus{$compare_menu}}) {
> # compare array A (@{$fvwm2_menus{$compare_menu}{$compkey}})
> to Array B (@{$fvwm2_menus{$reference_menu}{$refkey}})
> # and removing B elements from A. Exception: DestroyMenu and
> AddToMenu entires
> my @rest = grep { my $x = $_; not grep { $x =~ /\Q$_/i and $x
> !~ /DestroyMenu|AddToMenu/} @{$fvwm2_menus{$reference_menu}{$refkey}} }
> @{$fvwm2_menus{$compare_menu}{$compkey}};
This is why your structure is incorrect -- and leads to inefficiencies like
this. What you actually have in the abstract case is a structure such as
this:
# This is just me thinking...
%menu = (
'fvwm' => {
'name' => "mynicenewmenuname",
'items' => [
{
'label' => 'label4',
'action' => $some_action4,
'icon' => undef,
},
{
'label' => 'label3',
'action' => $some_action3,
'icon' => undef,
}
],
}
);
Note that you might have multiple item sections, perhaps where each section
has a separator line (+"" Nop) defined in it. How this translates from
{gnome,kde}- prefix menus, I don't know. It's down to you to find out.
Traversing this is slightly easier then, for you can now do things like:
my @menu_entries = map {
"+ ". $_->{'label'} . "\tPopup " . $_->{'action'} . ",\n";
} @{ $menu{'fvwm'}{'items'} };
This will allow to use functions like join as well as anything else. Note
the struture says nothing about the overall format for things like needing
{Add,Destroy}Menu lines. These can be added at the point you generate the
menu, and do not need to be stored as part of the menu's data. You need
only look at the rather hacked-up fvwm-convert-2.6 script to see examples of
this which I wrote.
The other benefit to the above is that traversing this also means you do not
end up grepping an array, but now can check a hash key which is O(1) and not
O(n*m) as in your current implementation.
Grepping/manipulating data which is already in the output format you're
after is almost always doomed to looping and ugly constructs because that
data is almost never in a parsable format to begin with.
HTH,
-- Thomas Adam
--
"Deep in my heart I wish I was wrong. But deep in my heart I know I am
not." -- Morrissey ("Girl Least Likely To" -- off of Viva Hate.)
