Item12180: Remove restrictions on feedback command placement. They wi…

…ll now be removed from check() output during page build (but not in response to a feedback event). Any discontiguous non-commands in check/pF output are coalesced (in order) into a single window replacement. All commands are executed after that replacement. This makes it easier for provideFeedback & checkers to cooperate, and for simple feedback to be provided from check(). Add DBG() checker output method - produces a <pre> block for delivering debugging messages. Updated Socket version in DEPENDENCIES. Deliver checker exception reports in a <pre> block so they are readable.

git-svn-id: http://svn.foswiki.org/trunk@16273 0b4bb1d4-4e5a-0410-9cc4-b2b747904278

core/lib/Foswiki/Configure/Checkers/AUDITGROUP.pm

@@ -232,23 +232,45 @@ sub _getFB {
 
     # Use only the status window updates.  If control updates or modal
     # data was generated, it has no place on this report.  (It is delivered
-    # when the status windows are updated.)
+    # when the status windows are updated.)  See Feedback::deliverResponse
+    # for structure of this data.
 
     foreach my $string (@$responses) {
-        if ( $string !~ s/\A\001// ) {    # Not encoded
-            $string = "}status\002$string";
-        }
-        my @items = split( '\001', $string );
-        foreach my $item (@items) {
+        my $txt = $string;
+
+        while (1) {
+            my ($len);
+            if ( $txt !~ s/\A\001// ) {
+
+               # Unencoded text (from NOTE, WARN, ERROR - or bare text)
+               # Length runs to next encoded block, or end of string & may be 0.
+                $len = index( $txt, "\001" );
+                if ( $len == -1 ) {
+                    $feedback .= $txt;
+                    last;
+                }
+                $feedback .= substr( $txt, 0, $len, '' );
+                next if ( length $txt );
+                last;
+            }
+
+            # Pre-encoded commands from FB_xxx
+            # Length, is used to find end of each command, and removed
+            die "Bad command string\n" unless ( $txt =~ s/\A(\d+),\{/{/ );
+            $len = $1;
+            die "Bad command length\n" unless ( $len && $len <= length $txt );
+            my $item = substr( $txt, 0, $len, '' );
+
             my ( $target, $action, $data ) =
-              split( /(\002|\003|\005)/, $item, 2 );
+              split( /(\002|\003|\005|\006)/, $item, 2 );
             $target ||= '';
             $action ||= '';
             $data   ||= '';
             if ( $action eq "\002" ) {
                 $feedback .= $data;
-                next;
             }
+            next if ( length $txt );
+            last;
         }
     }
     return $feedback;

core/lib/Foswiki/Configure/Feedback.pm

@@ -431,7 +431,7 @@ sub deliverResponse {
     my $fb      = shift;
     my $updated = shift;
 
-    my $response = '';
+    my $response;
 
 # Return encoded responses to each responding key.
 #
@@ -460,23 +460,67 @@ sub deliverResponse {
       '{ConfigureGUI}{Unsaved}' => Foswiki::unsavedChangesNotice($updated)
       if ( $updated && ( loggedIn($session) || $badLSC || $query->auth_type ) );
 
-    my $first = 1;
+    # Feedback is ordered so that commands from chained checkers execute in the
+    # specified order.  Internally, checkers can produce arbitrary mixes of
+    # text and command blocks.  The internal blocks contain a length field so
+    # they can be separated from any text that follows.  The text segments
+    # need to be coalesced into a single status window update, and the command
+    # blocks converted to the wire format (without lengths.)
+
     while ( @$fb >= 2 ) {
         my ( $keys, $txt ) = splice( @$fb, 0, 2 );
 
-        $response .= "\001" unless ($first);
-        if ( $txt =~ s/\A\001// ) {    # FB_FOR/FB_VALUE pre-encoded data
-            $response .= $txt;
+        my ( $text, $cmds );
+        while (1) {
+            my ($len);
+            if ( $txt !~ s/\A\001// ) {
+
+               # Unencoded text (from NOTE, WARN, ERROR - or bare text)
+               # Length runs to next encoded block, or end of string & may be 0.
+                $text = '' unless ( defined $text );
+                $len = index( $txt, "\001" );
+                if ( $len == -1 ) {
+                    $text .= $txt;
+                    last;
+                }
+                $text .= substr( $txt, 0, $len, '' );
+                next if ( length $txt );
+                last;
+            }
+
+            # Pre-encoded commands from FB_xxx
+            # Length, is used to find end of each command, and removed
+            die "Bad command string from $keys\n"
+              unless ( $txt =~ s/\A(\d+),\{/{/ );
+            $len = $1;
+            die "Bad command length from $keys\n"
+              unless ( $len && $len <= length $txt );
+            if ( defined $cmds ) {
+                $cmds .= "\001";
+            }
+            else {
+                $cmds = '';
+            }
+            $cmds .= substr( $txt, 0, $len, '' );
+            next if ( length $txt );
+            last;
+        }
+        die "Confused response from $keys"
+          unless ( defined $text || defined $cmds );
+        $response .= "\001" if ( defined $response );
+
+        if ( defined $text ) {
+            $response .= "$keys\002$text";
+            $response .= "\001$cmds" if ( defined $cmds );
         }
         else {
-            $response .= "$keys\002$txt";
+            $response .= $cmds;
         }
-        undef $first;
     }
     die "Invalid feedback list\n" if (@$fb);
 
-    $response .= "\177"
-      if ($first);    # no-data marker for client.  Really shouldn't happen.
+    $response = "\177"
+      unless ( defined $response );    # no-data marker for client.
 
     Foswiki::htmlResponse( $response, Foswiki::NO_REDIRECT() );
 
@@ -528,12 +572,14 @@ sub startVisit {
         $visitee->{_fbchecker} = my $checker = $visitee->{_fbchecker}
           || Foswiki::Configure::UI::loadChecker( $keys, $visitee );
 
-        # Multiple checks possible in audit AUDITGROUP, which limits buttons
-        # to 20.  The upper bound here is simply to catch a loop where a
+        # Multiple checks possible in audit AUDITGROUP.  The arbitrary
+        # upper bound here is simply to catch a loop where a
         # provider re-requests itself every time - or a depenency loop
-        # of n checkers that has the same effect.
+        # of n checkers that has the same effect.  We should never see
+        # anything like this number of visits to the same checker in a
+        # single event.
         die "$keys run loop\n"
-          if ( ++$this->{nrun}{$keys} > 21 );
+          if ( ++$this->{nrun}{$keys} > 100 );
 
         # See if it provides feedback.  If not, just re-check.
 
@@ -546,7 +592,8 @@ sub startVisit {
             };
             if ($@) {
                 $text = $checker->ERROR(
-                    "Feedback for $keys failed:  check for .spec issues: $@");
+"Feedback for $keys:$button failed:  check for .spec issues:  <pre>$@</pre>"
+                );
                 $checkall = 0;
             }
 
@@ -561,7 +608,8 @@ sub startVisit {
             my $check = eval { return $checker->check($visitee); };
             if ($@) {
                 $check = $checker->ERROR(
-                    "Checker for $keys failed: check for .spec issues:$@");
+"Feedback for $keys failed: check for .spec issues:  <pre>$@</pre>"
+                );
             }
             $check = '' unless ( defined $check );
             unless ( $check eq 'NOT USED IN THIS CONFIGURATION' ) {
@@ -596,7 +644,8 @@ sub startVisit {
             my $check = eval { return $checker->check($visitee); };
             if ($@) {
                 $check = $checker->ERROR(
-                    "Checker for $keys failed: check for .spec issues:$@");
+"Checker for $keys failed: check for .spec issues: <pre>$@</pre>"
+                );
             }
             $check = '' unless ( defined $check );
             unless ( $check eq 'NOT USED IN THIS CONFIGURATION' ) {

core/lib/Foswiki/Configure/UI.pm

@@ -446,14 +446,36 @@ sub ERROR {
 
 =begin TML
 
+---++ ObjectMethod DBG(...)
+
+Generate HTML for a debug message.
+
+These really should not appear in production, but during development,
+it is convenient to have a shorthand to produce preformatted text,
+e.g. when dumping data structures.
+
+=cut
+
+# Don't be tempted to rename DEBUG, as that is '0' in Assert...
+
+sub DBG {
+    my $this = shift;
+
+    return CGI::div( { class => 'configureDebug' },
+        CGI::span( CGI::strong('Debug ') . CGI::pre( join( "\n", @_ ) ) ) );
+}
+
+=begin TML
+
 ---++ ObjectMethod FB_FOR(...)
 
 Generate feedback for a named key (other than $this).
-Only usable in =provideFeedback= methods.
+Only meaningful in =provideFeedback= methods, but will
+be discarded if included in check() output.
+
  Usage: return $this->NOTE("My feedback")
                .$this->FB_FOR('{anotherkey}',
                               $this->WARN("That feedback" ));
-All FB_FOR clauses must follow any messages for $this.
 Feedback completly replaces the message area under an item,
 so multiple updates of a target will retain only the last encountered.
 
@@ -466,7 +488,8 @@ sub FB_FOR {
     my $target = eval "exists \$Foswiki::cfg$keys";
     die "Invalid FB_FOR target $keys\n" if ( $@ || !$target );
 
-    return "\001$keys\002" . join( "\n", @_ );
+    my $text = "$keys\002" . join( "\n", @_ );
+    return "\001" . length($text) . ",$text";
 }
 
 =begin TML
@@ -483,7 +506,8 @@ sub FB_GUI {
     my $this = shift;
     my $id   = shift;
 
-    return "\001$id\002" . join( "\n", @_ );
+    my $text = "$id\002" . join( "\n", @_ );
+    return "\001" . length($text) . ",$text";
 }
 
 =begin TML
@@ -509,7 +533,8 @@ sub FB_VALUE {
     my $target = eval "exists \$Foswiki::cfg$keys";
     die "Invalid FB_VALUE target $keys\n" if ($@);
 
-    return "\001$keys\003" . join( "\004", @_ );
+    my $text = "$keys\003" . join( "\004", @_ );
+    return "\001" . length($text) . ",$text";
 }
 
 =begin TML
@@ -524,7 +549,8 @@ sub FB_GUIVAL {
     my $this = shift;
     my $keys = shift;
 
-    return "\001$keys\003" . join( "\004", @_ );
+    my $text = "$keys\003" . join( "\004", @_ );
+    return "\001" . length($text) . ",$text";
 }
 
 =begin TML
@@ -554,7 +580,8 @@ sub FB_MODAL {
     my $this = shift;
     my $options = shift || 'r';
 
-    return "\001{ModalOptions}$options\005" . join( "", @_ );
+    my $text = "{ModalOptions}$options\005" . join( '', @_ );
+    return "\001" . length($text) . ",$text";
 }
 
 =begin TML
@@ -575,7 +602,8 @@ sub FB_ACTION {
     my $target  = shift;
     my $actions = shift;
 
-    return "\001{$target}$actions\006" . join( '', @_ );
+    my $text = "{$target}$actions\006" . join( '', @_ );
+    return "\001" . length($text) . ",$text";
 }
 
 =begin TML

core/lib/Foswiki/Configure/UIs/Value.pm

@@ -72,14 +72,46 @@ sub renderHtml {
 
     my $checker = Foswiki::Configure::UI::loadChecker( $keys, $value );
     if ($checker) {
-        eval { $check = $checker->check($value) || ''; };
+        my $output;
+        eval { $output = $checker->check($value); };
         if ($@) {
-            $check =
+            $output =
               $this->ERROR( "Checker ("
                   . ref($checker)
-                  . ") for $keys failed: check for .spec errors:$@" );
+                  . ") for $keys failed: check for .spec errors:  <pre>$@</pre>"
+              );
+        }
+        $output = '' unless ( defined $output );
+
+        # Remove any feedback command blocks - they don't apply to the static
+        # presentation created here.  This could be done in-place, but this
+        # is the same logic used in Feedback to reduce the risk of divergence.
+        while (1) {
+            my ($len);
+            if ( $output !~ s/\A\001// ) {
+
+               # Unencoded text (from NOTE, WARN, ERROR - or bare text)
+               # Length runs to next encoded block, or end of string & may be 0.
+                $len = index( $output, "\001" );
+                if ( $len == -1 ) {
+                    $check .= $output;
+                    last;
+                }
+                $check .= substr( $output, 0, $len, '' );
+                next if ( length $output );
+                last;
+            }
+
+            # Pre-encoded commands from FB_xxx
+            # Length, is used to find end of each command, and removed
+            die "Bad command string\n" unless ( $output =~ s/\A(\d+),\{/{/ );
+            $len = $1;
+            die "Bad command length\n"
+              unless ( $len && $len <= length $output );
+            substr( $output, 0, $len, '' );
+            next if ( length $output );
+            last;
         }
-        $check = '' unless ( defined $check );
         if ($check) {
 
             # something wrong

core/lib/Foswiki/Configure/resources/scripts.js

@@ -1052,7 +1052,12 @@ var feedback = ( function ($) {
                     items = data.split("\x01");
                     data = undefined;
                     for (item = 0; item < items.length; item++) {
-                        /* IE sometimes doesn't do capturing split, so simulate one. */
+                        /* IE sometimes doesn't do capturing split, so simulate one.
+                         * N.B. AUDITGROUP, Feedback, and UIs/Value also parse this data.
+                         * UI & Feeback generate it.  Protocol changes need to check each
+                         * for any required updates.  AUDITGROUP understands semantics;
+                         * the others structure.
+                         */
                         delims = ["\x02", "\x03","\x05", "\x06"];
                         for (i = 0; i < delims.length; i++) {
                             sloc = items[item].indexOf(delims[i]);

core/lib/Foswiki/Configure/resources/styles.css

@@ -572,6 +572,7 @@ div.configureUnsaved,
 div.configureNoChanges,
 div.configureError,
 div.configureWarn,
+div.configureDebug,
 div.configureOk {
 	padding:7px 1em 7px 3em;
 	background-position:1em .7em;
@@ -583,11 +584,14 @@ div.configureError {
 div.configureWarn {
 	background-image:url(%RESOURCEURI%icon_warn.png);
 }
+div.configureDebug {
+        background-image:url(%RESOURCEURI%icon_debug.png);
+}
 div.configureUnsaved {
-    background-image:url(%RESOURCEURI%icon_changes.png);
+        background-image:url(%RESOURCEURI%icon_changes.png);
 }
 div.configureNoChanges {
-    background-image:url(%RESOURCEURI%icon_nochanges.png);
+        background-image:url(%RESOURCEURI%icon_nochanges.png);
 }
 
 div.configureOk {

core/lib/Foswiki/Contrib/core/DEPENDENCIES

@@ -40,7 +40,7 @@ MIME::Base64,>=0,cpan,Required, for base Foswiki
 Mozilla::CA,>=20110904,cpan,Optional, SSL host verification for e-mail and other SSL/TLS connections
 Net::SMTP,>=2.29,cpan,Optional, required for sending mail if sendmail not available on server
 POSIX,>=1,cpan,Required, for base Foswiki
-Socket,>=0,cpan,Required, for base Foswiki
+Socket,>=2.001,cpan,Required, for base Foswiki
 Sort::Maker,0.06,cpan, Required, for base Foswiki
 Symbol,>=0,cpan,Optional, may be required for international characters
 Unicode::MapUTF8,>=0,cpan,Optional, This is required for international characters for Perl 5.6. Do not install this for Perl 5.8 or later.

core/lib/Foswiki/Contrib/core/MANIFEST

@@ -697,6 +697,7 @@ lib/Foswiki/Configure/resources/favicon.ico 0444
 lib/Foswiki/Configure/resources/foswiki-configure-logo.png 0444
 lib/Foswiki/Configure/resources/icon_changes.png 0444
 lib/Foswiki/Configure/resources/icon_nochanges.png 0444
+lib/Foswiki/Configure/resources/icon_debug.png 0444
 lib/Foswiki/Configure/resources/icon_down.gif 0444
 lib/Foswiki/Configure/resources/icon_error.png 0444
 lib/Foswiki/Configure/resources/icon_info.png 0444