split(/\|/, $hash->{REGEXP}) ist semantisch falsch

Begonnen von ska-, 08 Mai 2021, 22:04:01

Vorheriges Thema - Nächstes Thema

ska-

Hallo,

in notify.pm (und anderen, wie dem FileLog) kann man per addRegexpPart and removeRegexpPart Teile der mit | verketteten regex hinzufügen oder entfernen. Ausserdem wird die regex stets als /^$re$/ verwendet, weshalb im Wiki die Klammerung empfohlen wird: https://wiki.fhem.de/wiki/Notify#Einfache_ODER_Funktion . add/remove nutzt intern einen HASH, womit die Reihenfolge durcheinander geraten kann.

Problem 1) regex Device:brightness:.*(\d|1[0-4])
wird am | zerlegt, add/remove parts teilt an dieser Stelle, wenn nun die Reihenfolge geändert wird, ist die regex zerstört.

Problem 2) regex a|b|c|0
wird intern zu: ^a|b|0$
d.h. (nur) a ist am Anfang, b  nirgend und (nur) 0 am Ende gebunden.

Deshalb soll man laut Wiki
Problem 3) (a|b|c|0)
schreiben, was (bei mir im Test) nach remove "c" zu: (a|0)|b
wurde.


Folgender Code stellt sicher, dass immer nur an korrekten | geteilt wird. Leider wird in Problem 3 dann immer nur ein Part angezeigt.
BTW: IMHO sollte die regex statt als /^$re$/ mit /^(?:$re)$/ getestet werden, dann braucht man die Klammerung im Befehl nicht.

diff --git a/FHEM/91_notify.pm b/FHEM/91_notify.pm
index ca6bc22..9946c5b 100644
--- a/FHEM/91_notify.pm
+++ b/FHEM/91_notify.pm
@@ -171,6 +171,27 @@ notify_Attr(@)

###################################

+# Split RE into OR'ed parts
+sub re_split ($) {
+    my @re;
+    my @h = grep { length($_) > 0 } split(/\|/, $_[0]);
+    my $re = shift @h;
+    while(@h) {
+       eval { 'Hallo' =~ m/^($re)$/; };
+       if($@) {        # $re is not a valid regex
+               my $h = shift @h;
+               $re .= '|' . $h;
+       } else {
+               push @re, $re;
+               $re = shift @h;
+       }
+    }
+    push @re, $re;
+    return @re;
+}
+
+###################################
+
sub
notify_Set($@)
{
@@ -189,28 +210,26 @@ notify_Set($@)
   return "$cmd needs $sets{$cmd} parameter(s)" if(@a-$sets{$cmd} != 2);

   if($cmd eq "addRegexpPart") {
-    my %h;
     my $re = "$a[2]:$a[3]";
-    map { $h{$_} = 1 } split(/\|/, $hash->{REGEXP});
-    $h{$re} = 1;
-    $re = join("|", sort keys %h);
     return "Bad regexp: starting with *" if($re =~ m/^\*/);
-    eval { "Hallo" =~ m/^$re$/ };
+    eval { 'Hallo' =~ /^(?:$re)/; };
     return "Bad regexp: $@" if($@);
+    my @h = re_split($hash->{REGEXP});
+    push @h, $re unless grep { $_ eq $re } @h;
+    $re = join("|", @h);
     $hash->{REGEXP} = $re;
     $hash->{DEF} = "$re ".$hash->{".COMMAND"};
     notifyRegexpChanged($hash, $re);
     
   } elsif($cmd eq "removeRegexpPart") {
-    my %h;
-    map { $h{$_} = 1 } split(/\|/, $hash->{REGEXP});
-    return "Cannot remove regexp part: not found" if(!$h{$a[2]});
-    return "Cannot remove last regexp part" if(int(keys(%h)) == 1);
-    delete $h{$a[2]};
-    my $re = join("|", sort keys %h);
-    return "Bad regexp: starting with *" if($re =~ m/^\*/);
-    eval { "Hallo" =~ m/^$re$/ };
-    return "Bad regexp: $@" if($@);
+    my @h = re_split($hash->{REGEXP});
+    return "Cannot remove last regexp part" if(scalar(@h) == 1);
+    my @re = grep { $_ ne $a[2] } @h;
+    return "Cannot remove regexp part: not found" if scalar(@h) == scalar(@re);
+    my $re = join("|", @re);
+    return "Bad regexp: starting with *" if($re =~ m/^\*/);    # should not necessary
+    eval { "Hallo" =~ m/^$re$/ };      # should not necessary
+    return "Bad regexp: $@" if($@);    # should not necessary
     $hash->{REGEXP} = $re;
     $hash->{DEF} = "$re ".$hash->{".COMMAND"};
     notifyRegexpChanged($hash, $re);
@@ -249,7 +268,7 @@ notify_fhemwebFn($$$$)
   my $ret .= "<div class='makeTable wide'><span>Change wizard</span>".
              "<table class='block wide'>";
   my $row = 0;
-  my @ra = split(/\|/, $hash->{REGEXP});
+  my @ra = re_split($hash->{REGEXP});
   $ret .= "<tr class='".(($row++&1)?"odd":"even").
         "'><td colspan='2'>Change the condition:</td></tr>";
   if(@ra > 1) {


Ich denke, dieser Patch bindet jede regex ans Anfang / Ende:
diff --git a/FHEM/91_notify.pm b/FHEM/91_notify.pm
index 59f29a4..e2651b2 100644
--- a/FHEM/91_notify.pm
+++ b/FHEM/91_notify.pm
@@ -99,15 +99,15 @@ notify_Exec($$)
   for (my $i = 0; $i < $max; $i++) {
     my $s = $events->[$i];
     $s = "" if(!defined($s));
-    my $found = ($n =~ m/^$re$/ || "$n:$s" =~ m/^$re$/s);
+    my $found = ($n =~ m/^(?:$re)$/ || "$n:$s" =~ m/^(?:$re)$/s);
     if(!$found && AttrVal($n, "eventMap", undef)) {
       my @res = ReplaceEventMap($n, [$n,$s], 0);
       shift @res;
       $s = join(" ", @res);
-      $found = ("$n:$s" =~ m/^$re$/);
+      $found = ("$n:$s" =~ m/^(?:$re)$/);
     }
     if($found) {
-      next if($iRe && ($n =~ m/^$iRe$/ || "$n:$s" =~ m/^$iRe$/));
+      next if($iRe && ($n =~ m/^(?:$iRe)$/ || "$n:$s" =~ m/^(?:$iRe)$/));
       Log3 $ln, 5, "Triggering $ln";
       my %specials= (
                 "%NAME" => $n,