That’s actually not terribly horrible. If I were a potential user of it, I wouldn’t particularly itch to fix it up. (I am eternally cursed with my good grasp of the language when I go shopping for irssi scripts. I always have to spend a long time pummelling them into shape before I can bear to use them.)

I think setup_id can be written like this:

sub setup_id {
	my ( $event ) = @_;
	my $wid = $event->_win_id;
	return $byid{$wid} ||= WinData->new(
		wid    => $wid,
		icon_x => 0,
		icon_y => 0,
	);
}

Also, I’d write fvwm_quote like this:

sub fvwm_quote { map { s(")(\")g; s(\$)($$)g; $_ } my @copy = @_ }

That way you can pass in any number of strings to quote, and get them all back quoted, in one fell swoop.

The first line of your event handlers is unfortunately not really generalisable as Perl 5 so far does not have formal subroutine parameters. And generalising only the second line is too much effort to go through for this particular script.

Particularly since the whole setup_id function isn’t really necessary.

If I’d written it, I’d have used a simple hash of hashes instead of the WinData class; that’s about it. One nice thing about Perl (which has its downsides too occasionally, though I find them far outweighed by its convenience) is called autovivification. What that means is that in expressions on the left side of some form of assignment, intermediate levels of data structure will automatically be created for you if they don’t already exist: you can write $hash{$foo}{$bar}{$baz} = $quux even if $hash{$foo} doesn’t already exist. A new hash will automatically be assigned to it on your behalf, as well as subsequently to $hash{$foo}{$bar}, so that you assignment to $hash{$foo}{$bar}{$baz} can succeed. Therefor, you can write this:

$module->add_handler(M_WINDOW_NAME, sub {
	my ($self, $event) = @_;
	my $wi = setup_id($event);
	$wi->name($event->_name);
});

as just this (assuming you had a hash %wininfo and not %byid):

$module->add_handler(M_WINDOW_NAME, sub {
	my ($self, $event) = @_;
	$wininfo{$event->_win_id}{name} = $event->_name;
});

There is no need to manually make sure that a hash exists in $wininfo{$event->_win_id}, as one will automatically be created if not.

Modulo lack of the WinInfo class and setup_id function, and hash access rather than method calls in the M_END_WINDOWLIST handler, the script would be essentially the same.

If I did feel like using objects for that purpose I’d probably have used Object::Tiny::Lvalue… no one uses Class::Struct these days. I’d also move setup_id into WinInfo as a class method summon_for_event or such and would write the repetitive handlers as instance methods, with the event handlers using the former to delegate to one of the latter. That way you could boil down the repetitive handler bits to just this:

my %handler = (
	pull_frame_xy => M_CONFIGURE_WINDOW,
	pull_name     => M_WINDOW_NAME,
	# ...
);

while (my ($method, $mask) = each %handler) {
	$module->add_handler($mask, sub {
		my ($self, $event) = @_;
		WinInfo->summon_for($event)->$method($event);
	});
}

However. that’s just over-engineering for your case.

—Aristotle Pagaltzis

Wow, what a hammer. Have you considered using bookmarks? Either locally or in the cloud :)

The problem with bookmarks is that I ignore them even more than I already ignore all of those iconified windows. There is effectively no chance that I will ever read anything that I have saved in a file or bookmarked as 'to read someday'.

(Possibly this is my real problem, of course.)