Item12180: Improve documentation, reduce special cases and move out o…

…f mainline. Improve dislay of undefined values. git-svn-id: http://svn.foswiki.org/trunk@16328 0b4bb1d4-4e5a-0410-9cc4-b2b747904278
foswiki · Jan 2, 2013 · 85862ea · 85862ea
1 parent 73199d3
commit 85862ea
Show file tree

Hide file tree

Showing 6 changed files with 85 additions and 33 deletions.
diff --git a/core/lib/Foswiki/Configure/Checkers/ConfigureGUI/Modals/SaveChanges.pm b/core/lib/Foswiki/Configure/Checkers/ConfigureGUI/Modals/SaveChanges.pm
@@ -26,6 +26,8 @@ sub generateForm {
     my $updated = { %{ $this->{item}{_fbChanged} } };
 
     my $cart = Foswiki::Configure::Feedback::Cart->get($session);
+    my $passChanged = ( defined $cart->param('{Password}') ) ? 1 : 0;
+    $query->param( 'TYPEOF:{Password}', 'PASSWORD' );
 
     my $changesList = [];
     foreach my $key ( sortHashkeyList( keys %$updated ) ) {
@@ -35,23 +37,30 @@ sub generateForm {
         }
         my $valueString;
         my $type = $query->param("TYPEOF:$key") || 'UNKNOWN';
-        if ( $key =~ /password/i ) {
-            $valueString = '&bull;' x 9;
+        if ( $type eq 'PASSWORD' ) {
+            $valueString = '&bull;' x 15;
         }
         elsif ( $type eq 'BOOLEAN' ) {
             $valueString = $query->param($key) ? 1 : 0;
         }
         else {
-            $valueString = join( ',', $query->param($key) );
+            my $ek = $key;
+            $ek =~ s/\}$/_}/;
+            if ( $query->param("TYPEOF:$ek") && !$query->param($ek) ) {
+                $valueString =
+                  '<span class="configureUndefinedValue">undefined</span>';
+            }
+            else {
+                $valueString = join( ', ', $query->param($key) );
+            }
         }
         push( @$changesList, { key => $key, value => $valueString } );
     }
+    $query->delete('TYPEOF:{Password}');
     my $modified = @$changesList;
     my @items;
     @items = sortHashkeyList( keys %$updated ) if $modified;
 
-    my $passChanged = ( defined $cart->param('{Password}') ) ? 1 : 0;
-
     $template->addArgs(
         items         => \@items,
         changesList   => $changesList,
@@ -154,6 +163,8 @@ sub processForm {
 
     # Build list of hashes with each changed key and its value(s) for template
 
+    $query->param( 'TYPEOF:{Password}', 'PASSWORD' );
+
     my $changesList = [];
     foreach my $key ( sortHashkeyList( keys %$updated ) ) {
         if ( $key =~ /^\{ConfigureGUI\}/ ) {
@@ -162,17 +173,26 @@ sub processForm {
         }
         my $valueString;
         my $type = $query->param("TYPEOF:$key") || 'UNKNOWN';
-        if ( $key =~ /password/i ) {
-            $valueString = '&bull;' x 9;
+        if ( $type eq 'PASSWORD' ) {
+            $valueString = '&bull;' x 15;
         }
         elsif ( $type eq 'BOOLEAN' ) {
             $valueString = $query->param($key) ? 1 : 0;
         }
         else {
-            $valueString = join( ',', $query->param($key) );
+            my $ek = $key;
+            $ek =~ s/\}$/_}/;
+            if ( $query->param("TYPEOF:$ek") && !$query->param($ek) ) {
+                $valueString =
+                  '<span class="configureUndefinedValue">undefined</span>';
+            }
+            else {
+                $valueString = join( ', ', $query->param($key) );
+            }
         }
         push( @$changesList, { key => $key, value => $valueString } );
     }
+    $query->delete('TYPEOF:{Password}');
     my $modified = @$changesList;
 
     push @$changesList, { key => 'No configuration items changed', value => '' }

diff --git a/core/lib/Foswiki/Configure/Checkers/ConfigureGUI/Modals/UnsavedDetail.pm b/core/lib/Foswiki/Configure/Checkers/ConfigureGUI/Modals/UnsavedDetail.pm
@@ -38,23 +38,33 @@ sub generateForm {
 
     my $cart = Foswiki::Configure::Feedback::Cart->get($session);
     my $passChanged = $cart->param('{Password}') ? 1 : 0;
+    $query->param( 'TYPEOF:{Password}', 'PASSWORD' );
 
     my $pendingItems = [];
     foreach my $key ( sortHashkeyList( keys %$updated ) ) {
         next if ( $key =~ /^\{ConfigureGUI\}/ );
         my $valueString;
         my $type = $query->param("TYPEOF:$key") || 'UNKNOWN';
-        if ( $key =~ /password/i ) {
-            $valueString = '&bull;' x 9;
+        if ( $type eq 'PASSWORD' ) {
+            $valueString = '&bull;' x 15;
         }
         elsif ( $type eq 'BOOLEAN' ) {
             $valueString = $query->param($key) ? 1 : 0;
         }
         else {
-            $valueString = join( ',', $query->param($key) );
+            my $ek = $key;
+            $ek =~ s/\}$/_}/;
+            if ( $query->param("TYPEOF:$ek") && !$query->param($ek) ) {
+                $valueString =
+                  '<span class="configureUndefinedValue">undefined</span>';
+            }
+            else {
+                $valueString = join( ', ', $query->param($key) );
+            }
         }
         push( @$pendingItems, { item => $key, value => $valueString } );
     }
+    $query->delete('TYPEOF:{Password}');
 
     $template->addArgs(
         pendingCount    => scalar @$pendingItems,

diff --git a/core/lib/Foswiki/Configure/FoswikiCfg.pm b/core/lib/Foswiki/Configure/FoswikiCfg.pm
@@ -805,17 +805,12 @@ sub startVisit {
 
     my $value = $this->{valuer}->currentValue($visitee);
 
-    if ( $this->{logger} ) {
-        my $logValue =
-            $typeName eq 'PASSWORD' ? '*' x 15
-          : defined $value ? $visitee->asString( $this->{valuer} )
-          :                  '<--undefined-->';
-        $this->{logger}->logChange( $visitee->getKeys(), $logValue );
-    }
-
+    my $logValue;
     if ( defined $value ) {
         my $type = $visitee->getType;
-        my ( $txt, $require ) = $type->value2string( $keys, $value );
+        $logValue = $visitee->asString( $this->{valuer} )
+          if ( $this->{logger} );
+        my ( $txt, $require ) = $type->value2string( $keys, $value, $logValue );
         if ( defined $require ) {
             if ( ref $require ) {
                 $this->{requires}{$_} = 1 foreach (@$require);
@@ -833,9 +828,13 @@ s/^\s*\$(?:Foswiki::)?cfg$keys\s*=.*?;\n/_updateEntry($keys,$txt)/msge
           );
     }
     else {
+        $logValue = '<--undefined-->';
         $this->{content} =~ s/^\s*?\$(?:Foswiki::)?cfg$keys\s*=.*?;\n//msg;
     }
 
+    $this->{logger}->logChange( $keys, $logValue )
+      if ( $this->{logger} );
+
     return 1;
 }
 

diff --git a/core/lib/Foswiki/Configure/Type.pm b/core/lib/Foswiki/Configure/Type.pm
@@ -143,6 +143,9 @@ are run through this method before being saved in the value store.
 It should *not* be used to do validation - use a Checker to do that, or
 JavaScript invoked from the prompt.
 
+This method is NOT invoked when values are read from LocalSite.cfg;
+it is not (likely) the inverse of value2string.
+
 =cut
 
 sub string2value {
@@ -152,26 +155,40 @@ sub string2value {
 
 =begin TML
 
----++ ObjectMethod value2string($value) -> ($string, $require)
+---++ ObjectMethod value2string($keys, $value, $log) -> ($string, $require)
 Used to encode output values during save.  The default is
 adequate for most value types, but this hook allows for special
 encoding when needed.  See PASSWORD for an example.
 
-$value - the value to be encoded (should not be undef; that's
-         filtered earlier.
+   * $keys - the {key}{s} of the value being output.
+
+   * $value - the value to be encoded.  This is the actual value
+              of the item, NOT a Foswiki::Configure::Value object.
+              It should not be undef; that is filtered earlier.
+
+   * $logValue - String value logged by default.  May be modified.  undef
+            if no logging will be done.
 
-$require - the name of a require module that's required for decoding
-          the value.  (LSC will contain require $require;)
-          Can be an arrayref [qw/mod1 mod2/].  undef is acceptable.
+   * $string - the text to be entered in LocalSite.cfg for this value.
+               For save to work, it must be in the form
+               $Foswiki::cfg{key}{s} = ...;\n
 
-This is mechanism is intended for exceptional cases.  This default
+   * $require - the name of a require module that's required for decoding
+               the value.  (LSC will contain require $require;)
+               Can be an arrayref [qw/mod1 mod2/].  Optional.
+
+This mechanism is intended for exceptional cases.  This default
 method should be adequate for virtually every item type.
 
+Do not confuse this method with others used to produce values for
+human consumption.  Do not assume that it is the inverse of
+string2value.
+
 =cut
 
 sub value2string {
     my $this = shift;
-    my ( $keys, $value ) = @_;
+    my ( $keys, $value, $log ) = @_;
 
     # For some reason Data::Dumper ignores the second parameter sometimes
     # when -T is enabled, so have to do a substitution

diff --git a/core/lib/Foswiki/Configure/Types/PASSWORD.pm b/core/lib/Foswiki/Configure/Types/PASSWORD.pm
@@ -31,13 +31,15 @@ sub prompt {
 # Encryption would be nice, but there's no place to store
 # the decryption password, and consumers need plaintext.
 #
-# Return:
-#   replacement text ($Foswiki::cfg{key}{s} = expression
-#   module for LSC to require for decode. (or arrayref if list)
+# Modify $logValue to keep passwords out of the logfiles.
+#
+# See Type.pm for API.
 
 sub value2string {
     my $this = shift;
-    my ( $keys, $value ) = @_;
+    my ( $keys, $value, $logValue ) = @_;
+
+    $_[2] = '*' x 15;
 
     my $txt = Data::Dumper->Dump( [ encode_base64( $value, '' ) ] );
     $txt =~

diff --git a/core/lib/Foswiki/Configure/resources/styles.css b/core/lib/Foswiki/Configure/resources/styles.css
@@ -190,6 +190,11 @@ body.configureShowOneSection .configureNotShowSection {
 	background-image:url(%RESOURCEURI%icon_error.png);
 }
 
+.configureUndefinedValue {
+    color:gray;
+    font-style:italic;
+}
+
 table.configureFeedbackSummaryTable {
     margin-top: 0px;
     border: none;
@@ -247,7 +252,6 @@ span.configureFeedbackPendingMessage {
     color:green;
     font-style:italic;
 }
-
 div.configureFeedbackError H1 {
     color: #FF0000;
     font-weight: bold;