Script replaces instead of add

geekism has asked for the wisdom of the Perl Monks concerning the following question:

I'm writing a irssi script, not sure if this is the place.. But when i do: /delay add nickname message, it adds it correctly. but if i do it twice. it over writes the first message, instead of creating another message. the purpose of the script is to send a DELAYED message to a user so when they sign on, it automatically sends the queued messages so i dont have to wait around for them to send it, below is the full script:

use strict;
no warnings;
use feature qw/switch/;
use Irssi;
 
our $VERSION = '0.10';
our %IRSSI = (
    author      =>  'black@justla.me',
    name        =>  'delayer.pl',
    todo        =>  'NOTHING',
);
 
my %delaylist = ();
 
sub delayer {
    my ($server, $nick, $user, $host, $real_name, $away_msg) = @_;
    if ($delaylist{$nick}) {
        $server->command("query $nick $delaylist{$nick}");
        removedelays($nick);
    }
}
 
sub delaycmd {
    my $command = shift;
    if (substr($command, 0, 1) eq '-') { $command = substr($command, 1
+) }
    my ($option, $args) = split(' ', $command, 2);
 
    if (!$option) {
        listdelays();
        return;
    }
 
    given ("$option") {
        when (/^list$/i)                { listdelays() }
        when (/^add$/i)                 { ($args) ? adddelays($args) :
+ usage() }
        when (/^remove$/i || /^rem$/i)  { removedelays($args) ; print 
+"$args" }
        default                         { usage() }
    }
   
}
 
sub listdelays {
    my $nicklist = shift;
    my @nicks;
    if (!$nicklist) {
        @nicks = keys %delaylist;
    } else { @nicks = split(' ', $nicklist)
    }
    if (!@nicks) {
        print "\cBdelayER\cB: delay list is empty.";
        return;
    }
 
    print "\cBdelayER\cB";
    foreach (@nicks) {
        print "--> \cB$_\cB: $delaylist{ $_ }";
    }
}
 
sub adddelays {
    my $addcmd = shift;
    my ($nicklist, $msg) = split(' ', $addcmd, 2);
    if (!$msg) {
        usage();
        return;
    }
    my @nicks = split(',', $nicklist);
    my %hash = map { $_, 1 } @nicks;
    @nicks = keys %hash;
    if (keys %delaylist) {
        my @isect = ();
        map { $hash{$_} = 1 } keys %delaylist;
        @isect = grep { $hash{$_} } @nicks;
        if (@isect) {
#            removedelays(@isect);
        }
    }
    foreach (@nicks) {
        $delaylist{ $_ } = $msg;
    }
    print "\cBdelayER\cB: " . scalar(@nicks) . ' delays added.';
}
 
sub removedelays {
    my $nicklist = shift;
    if (!$nicklist) {
        %delaylist = ();
        print "\cBdelayER\cB: All delays removed.";
    } else {
        foreach (split(' ', $nicklist)) {
            delete $delaylist{ $_ };
        }
        print "\cBdelayER\cB: " . scalar(split(' ', $nicklist)) .
            ' delays removed.';
    }
}
 
sub usage {
    print <<EOF
 
\cBdelayER\cB
Send a private message to a person on your /NOTIFY list when they join
+ the server.
 
Usage:
/delay (-add|add) nick[,nick...] message
    Add a delay for a nick, or a comma-delimited list of nicks.
    One delay per nick! Existing delays will be replaced without warni
+ng.
/delay (-remove|remove|rem) [nick [nick...]]
    Remove a delay targeted at a nick, or space-delimited list of nick
+s.
    If no nick is supplied, all delays will be removed without warning
+.
/delay [-list|list]
    Display a list of delays which have been set.
 
    Anything else will display this text.
EOF
}
 
Irssi::signal_add('notifylist joined', 'delayer');
Irssi::command_bind('delay', 'delaycmd');
Irssi::command_set_options('delay','add remove list help usage');
[download]

Comment on Script replaces instead of add Download Code

Replies are listed 'Best First'.
Re: Script replaces instead of add by roboticus (Chancellor) on Jul 23, 2015 at 04:12 UTC
geekism: It looks like adddelays is using a hash to store the messages: each nickname gets a single message. If you instead use a hash of arrays, you could have multiple messages for each nickname. It may make delay management a bit trickier, though--do you want to remove one delayed message from a nickname, or all. If one, which one? ...roboticus When your only tool is a hammer, all problems look like your thumb.	[reply]
Re^2: Script replaces instead of add by geekism (Initiate) on Jul 23, 2015 at 04:34 UTC
My goal is to ADD to the current hash. the way it is now. is it over-writes the entry for 'nick', instead of adding TO the queue. If a output of it in action is required, i can paste one.	[reply]
Re^3: Script replaces instead of add by roboticus (Chancellor) on Jul 23, 2015 at 10:27 UTC
OK, then, you could also try changing the function to add the new delayed message to the one you currently have. As I mentioned, you're simply replacing the current value in the hash. If you want to add it to the nickname, then you'll have to pull out the current message (if any) and combine it with the new message, and only then assign the value to the hash. ...roboticus When your only tool is a hammer, all problems look like your thumb.	[reply]
Re: Script replaces instead of add by soonix (Chancellor) on Jul 23, 2015 at 07:29 UTC
As roboticus pointed out, this: `$delaylist{ $_ } = $msg;` [download] essentially says one message per nick. You could replace it with something like `$delaylist{ $_ } .= $msg;` [download] which adds to the (still one) message (perhaps should add a linefeed or some such), or `push @{$delaylist{ $_ }}, $msg;` [download] which says multiple messages per nick - housekeeping left as an exercise for the reader ;-) In the latter case, you might want to change the semantics of delay-remove to remove all messages the first message the last message to that nick. (TL;DR essentially the same as this)	[reply] [d/l] [select]
Re^2: Script replaces instead of add by geekism (Initiate) on Jul 24, 2015 at 03:06 UTC
Using the above suggestions, appends to the prior nick: message, which i was already able to do. i need it to create a NEW message.. `For example (current code) nick message /delay add nick message nick message message. Expected results /delay list nick message /delay add nick message /delay list nick message nick message` [download]	[reply] [d/l]
Re^3: Script replaces instead of add by soonix (Chancellor) on Jul 24, 2015 at 08:22 UTC
If I understand you correctly, the problem is now in your `delaycmd` and `listdelay` subs. instead of `$server->command("query $nick $delaylist{$nick}");` [download] which sends the whole kaboodle as one message, make it `$server->command("query $nick $_") for @{$delaylist{$nick}};` [download] (in case you changed your delaylist from hash of strings to HoA). Similiar for `listdelays`. Something like `foreach (@nicks) { print "--> \cB$_\cB:\n"; print scalar @{ $delaylist{ $_ }} . " message(s)\n"; print "$_\n" for @{ $delaylist{ $_ }}; }` [download] BTW to me, it looks like the part with `my %hash = ...` and `@isect` was superfluous from the beginning. you should remove it, because otherwise it will distract you when you have the next change :-)	[reply] [d/l] [select]
Re^4: Script replaces instead of add by geekism (Initiate) on Jul 24, 2015 at 09:38 UTC
Re^5: Script replaces instead of add by soonix (Chancellor) on Jul 24, 2015 at 09:50 UTC
Re^3: Script replaces instead of add by soonix (Chancellor) on Jul 24, 2015 at 09:26 UTC
Alternative to my previous reply, if you don't want to "dive into" Perl Data Structures: The problem statement turns into: "how to represent multiple messages?" The most flexible is: as an array of individual messages. However, if you are certain there won't be, e.g., newlines in your messages, you can use that as a delimiter. That would make: In `adddelays`: `$delaylist{ $_ } .= $msg . "\n";` [download] In `listdelays`: `foreach (@nicks) { print "--> \cB$_\cB:\n"; print "$_\n" for split /\n/, $delaylist{ $_ }; }` [download] and finally, in `delayer`: `print ("query $nick $_") for split /\n/, $delaylist{$nick};` [download] Luckily, split omits trailing empty fields, otherwise the code in `adddelays` would have to be more complicated. Thanks perl for autovivification and DWIM :-)	[reply] [d/l] [select]