Re: Reworked fvwm-menu-desktop
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"
> >Gesendet: 20.10.2011 15:00:58
> >An: "Thomas Funk"
> >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.)
Re: Reworked fvwm-menu-desktop
>-Ursprüngliche Nachricht-
>Von: "Thomas Adam"
>Gesendet: 20.10.2011 15:00:58
>An: "Thomas Funk"
>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.
Hope it's ok:
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 = '';
} else {
# get the biggest menu as reference
foreach my $prefix (keys %fvwm2_menus) {
my $size = scalar keys %{$fvwm2_menus{$prefix}};
if ($size > $weight) {
$reference_menu = $prefix;
$weight = $size;
}
}
}
}
## 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}};
# clear compared array
undef @{ $fvwm2_menus{$compare_menu}{$compkey} };
# push cleared array into compare array
push @{ $fvwm2_menus{$compare_menu}{$compkey} }, @rest;
# if only DestroyMenu or AddToMenu entries exist, delete this
menu name hash
if (scalar @{ $fvwm2_menus{$compare_menu}{$compkey} } == 2) {
delete $fvwm2_menus{$compare_menu}{$compkey};
}
}
}
## check if double entries in the compare menu exist and delete them
# this happens on OpenSuSE 11.4 where in System AND Settings the same
app entries exist
foreach my $refkey (keys %{$fvwm2_menus{$compare_menu}}) {
foreach my $compkey (keys %{$fvwm2_menus{$compare_menu}}) {
next if ($compkey eq $refkey);
my @rest = grep { my $x = $_; not grep { $x =~ /\Q$_/i and $x
!~ /DestroyMenu|AddToMenu/} @{$fvwm2_menus{$compare_menu}{$refkey}} }
@{$fvwm2_menus{$compare_menu}{$compkey}};
undef @{ $fvwm2_menus{$compare_menu}{$compkey} };
push @{ $fvwm2_menus{$compare_menu}{$compkey} }, @rest;
}
# if only DestroyMenu or AddToMenu entries exist in a reference
menu, delete the menu name hash
if (scalar @{ $fvwm2_menus{$compare_menu}{$refkey} } == 2) {
delete $fvwm2_menus{$compare_menu}{$refkey};
}
}
## merge all '+' entries in existing ref menus
foreach my $key (keys %{$fvwm2_menus{$compare_menu}}) {
# don't check the fvwm root menu. This we will do later
next if ($key eq 'FvwmMenu');
# if menu name from compare menu exists in reference menu try
merging
# question: is defined correct to disable autovivification?
if (defined $fvwm2_menus{$reference_menu}{$key}) {
# get all + entries except with Popup|DestroyMenu|AddToMenu
my @entries = (grep (/\+/,
@{$fvwm2_menus{$compare_menu}{$key}}) and grep (!/Popup|DestroyMenu|AddToMenu/,
@{$fvwm2_menus{$compare_menu}{$key}}));
# if entries array has values, push lines into reference menu
if (@entries) {
push @{ $fvwm2_menus{$reference_menu}{$key} }, @entries;
# get the rest like Popup|DestroyMenu|AddToMenu entries to
create the
# compare menu without the + entries moved to the reference
my @rest = grep (/Popup|De
