From d96aba4fe41f070035972b022a1a0c24e81a5ec0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C4=8Ciha=C5=99?= Date: Thu, 3 May 2012 10:58:34 +0200 Subject: [PATCH 1/9] Coding style, translatable messages --- libraries/Advisor.class.php | 43 +++++++++++++++++++++++-------------- 1 file changed, 27 insertions(+), 16 deletions(-) diff --git a/libraries/Advisor.class.php b/libraries/Advisor.class.php index 80559a7c65..70cf3088b8 100644 --- a/libraries/Advisor.class.php +++ b/libraries/Advisor.class.php @@ -314,10 +314,10 @@ class Advisor ); $numRules = count($ruleSyntax); $numLines = count($file); - $j = -1; + $ruleNo = -1; $ruleLine = -1; - for ($i = 0; $i<$numLines; $i++) { + for ($i = 0; $i < $numLines; $i++) { $line = $file[$i]; if ($line[0] == '#' || $line[0] == "\n") { continue; @@ -326,27 +326,35 @@ class Advisor // Reading new rule if (substr($line, 0, 4) == 'rule') { if ($ruleLine > 0) { - $errors[] = 'Invalid rule declaration on line ' . ($i+1) - . ', expected line ' . $ruleSyntax[$ruleLine++] - . ' of previous rule' ; + $errors[] = sprintf( + __('Invalid rule declaration on line %1$s, expected line %2$s of previous rule'), + $i + 1, + $ruleSyntax[$ruleLine++] + ); continue; } if (preg_match("/rule\s'(.*)'( \[(.*)\])?$/", $line, $match)) { $ruleLine = 1; - $j++; - $rules[$j] = array( 'name' => $match[1]); - $lines[$j] = array( 'name' => $i + 1); + $ruleNo++; + $rules[$ruleNo] = array('name' => $match[1]); + $lines[$ruleNo] = array('name' => $i + 1); if (isset($match[3])) { - $rules[$j]['precondition'] = $match[3]; - $lines[$j]['precondition'] = $i + 1; + $rules[$ruleNo]['precondition'] = $match[3]; + $lines[$ruleNo]['precondition'] = $i + 1; } } else { - $errors[] = 'Invalid rule declaration on line '.($i+1); + $errors[] = sprintf( + __('Invalid rule declaration on line %s'), + $i + 1 + ); } continue; } else { if ($ruleLine == -1) { - $errors[] = 'Unexpected characters on line '.($i+1); + $errors[] = sprintf( + __('Unexpected characters on line %s'), + $i + 1 + ); } } @@ -357,12 +365,15 @@ class Advisor } // Non tabbed lines are not if ($line[0] != "\t") { - $errors[] = 'Unexpected character on line '.($i+1).' - . Expected tab, but found \''.$line[0].'\''; + $errors[] = sprintf( + __('Unexpected character on line %1$s. Expected tab, but found "%2$s"', + $i + 1, + $line[0] + ); continue; } - $rules[$j][$ruleSyntax[$ruleLine]] = chop(substr($line, 1)); - $lines[$j][$ruleSyntax[$ruleLine]] = $i + 1; + $rules[$ruleNo][$ruleSyntax[$ruleLine]] = chop(substr($line, 1)); + $lines[$ruleNo][$ruleSyntax[$ruleLine]] = $i + 1; $ruleLine += 1; } From 3912fd0c96ae2b6c18b7b5bfbb66a578a275042d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C4=8Ciha=C5=99?= Date: Thu, 3 May 2012 11:04:11 +0200 Subject: [PATCH 2/9] Fix typo, wrap long lines --- libraries/Advisor.class.php | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/libraries/Advisor.class.php b/libraries/Advisor.class.php index 70cf3088b8..b7f8279c2a 100644 --- a/libraries/Advisor.class.php +++ b/libraries/Advisor.class.php @@ -259,9 +259,10 @@ class Advisor * Runs a code expression, replacing variable names with their respective * values * - * @param string $expr expressoin to evaluate - * @param int $ignoreUntil if > 0, it doesn't replace any variables until that string - * position, but still evaluates the whole expr + * @param string $expr expression to evaluate + * @param int $ignoreUntil if > 0, it doesn't replace any variables until + * that string position, but still evaluates the + * whole expr * * @return result of evaluated expression */ From 735c1ebec70d04444f443d1c901e627e66f60c80 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C4=8Ciha=C5=99?= Date: Thu, 3 May 2012 11:08:00 +0200 Subject: [PATCH 3/9] Document expression evaluating a bit --- libraries/Advisor.class.php | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/libraries/Advisor.class.php b/libraries/Advisor.class.php index b7f8279c2a..73678c4446 100644 --- a/libraries/Advisor.class.php +++ b/libraries/Advisor.class.php @@ -239,15 +239,27 @@ class Advisor $this->runResult[$type][] = $rule; } + /** + * Callback for evaluating fired() condition. + * + * @param $matches array List of matched elements form preg_replace_callback + * + * @return Replacement value + */ private function ruleExprEvaluate_var1($matches) { - // '/fired\s*\(\s*(\'|")(.*)\1\s*\)/Uie' return '1'; //isset($this->runResult[\'fired\'] } + /** + * Callback for evaluating variables in expression. + * + * @param $matches array List of matched elements form preg_replace_callback + * + * @return Replacement value + */ private function ruleExprEvaluate_var2($matches) { - // '/\b(\w+)\b/e' return isset($this->variables[$matches[1]]) ? (is_numeric($this->variables[$matches[1]]) ? $this->variables[$matches[1]] @@ -272,11 +284,13 @@ class Advisor $exprIgnore = substr($expr, 0, $ignoreUntil); $expr = substr($expr, $ignoreUntil); } + // Evaluate fired() conditions $expr = preg_replace_callback( '/fired\s*\(\s*(\'|")(.*)\1\s*\)/Ui', array($this, 'ruleExprEvaluate_var1'), $expr ); + // Evaluate variables $expr = preg_replace_callback( '/\b(\w+)\b/', array($this, 'ruleExprEvaluate_var2'), @@ -288,10 +302,13 @@ class Advisor $value = 0; $err = 0; + // Actually evaluate the code ob_start(); eval('$value = '.$expr.';'); $err = ob_get_contents(); ob_end_clean(); + + // Error handling if ($err) { throw new Exception( strip_tags($err) . '
Executed code: $value = ' . $expr . ';' From 03cbe523b2d7c31c03b1b11c9268bcb9ded709a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C4=8Ciha=C5=99?= Date: Thu, 3 May 2012 11:10:56 +0200 Subject: [PATCH 4/9] Better name for methods --- libraries/Advisor.class.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/libraries/Advisor.class.php b/libraries/Advisor.class.php index 73678c4446..bdae2a0eee 100644 --- a/libraries/Advisor.class.php +++ b/libraries/Advisor.class.php @@ -246,7 +246,7 @@ class Advisor * * @return Replacement value */ - private function ruleExprEvaluate_var1($matches) + private function ruleExprEvaluate_fired($matches) { return '1'; //isset($this->runResult[\'fired\'] } @@ -258,7 +258,7 @@ class Advisor * * @return Replacement value */ - private function ruleExprEvaluate_var2($matches) + private function ruleExprEvaluate_variable($matches) { return isset($this->variables[$matches[1]]) ? (is_numeric($this->variables[$matches[1]]) @@ -287,13 +287,13 @@ class Advisor // Evaluate fired() conditions $expr = preg_replace_callback( '/fired\s*\(\s*(\'|")(.*)\1\s*\)/Ui', - array($this, 'ruleExprEvaluate_var1'), + array($this, 'ruleExprEvaluate_fired'), $expr ); // Evaluate variables $expr = preg_replace_callback( '/\b(\w+)\b/', - array($this, 'ruleExprEvaluate_var2'), + array($this, 'ruleExprEvaluate_variable'), $expr ); if ($ignoreUntil > 0) { From 2848a492cf06008900404c9cbf586766a669817a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C4=8Ciha=C5=99?= Date: Thu, 3 May 2012 11:19:23 +0200 Subject: [PATCH 5/9] Fix syntax error --- libraries/Advisor.class.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/Advisor.class.php b/libraries/Advisor.class.php index bdae2a0eee..dd28c0daee 100644 --- a/libraries/Advisor.class.php +++ b/libraries/Advisor.class.php @@ -384,7 +384,7 @@ class Advisor // Non tabbed lines are not if ($line[0] != "\t") { $errors[] = sprintf( - __('Unexpected character on line %1$s. Expected tab, but found "%2$s"', + __('Unexpected character on line %1$s. Expected tab, but found "%2$s"'), $i + 1, $line[0] ); From 48a7269acb29e549a8e8468eef72d911e99e45f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C4=8Ciha=C5=99?= Date: Thu, 3 May 2012 11:19:34 +0200 Subject: [PATCH 6/9] Real processing of fired condition --- libraries/Advisor.class.php | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/libraries/Advisor.class.php b/libraries/Advisor.class.php index dd28c0daee..a5d2794189 100644 --- a/libraries/Advisor.class.php +++ b/libraries/Advisor.class.php @@ -216,6 +216,7 @@ class Advisor } else { $rule['justification'] = $this->translate($rule['justification']); } + $rule['id'] = $rule['name']; $rule['name'] = $this->translate($rule['name']); $rule['issue'] = $this->translate($rule['issue']); @@ -248,7 +249,19 @@ class Advisor */ private function ruleExprEvaluate_fired($matches) { - return '1'; //isset($this->runResult[\'fired\'] + // No list of fired rules + if (!isset($this->runResult['fired'])) { + return '0'; + } + + // Did matching rule fire? + foreach ($this->runResult['fired'] as $rule) { + if ($rule['id'] == $matches[2]) { + return '1'; + } + } + + return '0'; } /** From b968b468b661063e97d7ff9f2f97f9aee422618a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C4=8Ciha=C5=99?= Date: Thu, 3 May 2012 11:21:46 +0200 Subject: [PATCH 7/9] Fix typo in fired() condition --- libraries/advisory_rules.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/advisory_rules.txt b/libraries/advisory_rules.txt index c204147d88..2e613b7bb5 100644 --- a/libraries/advisory_rules.txt +++ b/libraries/advisory_rules.txt @@ -285,7 +285,7 @@ rule 'Max % MyISAM key buffer ever used' [!PMA_DRIZZLE && key_buffer_size > 0] max % MyISAM key buffer ever used: %s%, this value should be above 95% | round(value,1) # Don't fire if above rule fired - we don't need the same advice twice -rule 'Percentage of MyISAM key buffer used' [!PMA_DRIZZLE && key_buffer_size > 0 && !fired('max % MyISAM key buffer ever used')] +rule 'Percentage of MyISAM key buffer used' [!PMA_DRIZZLE && key_buffer_size > 0 && !fired('Max % MyISAM key buffer ever used')] ( 1 - Key_blocks_unused * key_cache_block_size / key_buffer_size) * 100 value < 95 MyISAM key buffer (index cache) % used is low. From f724016f0246be6f4bafc886d82e47a079449bdf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C4=8Ciha=C5=99?= Date: Thu, 3 May 2012 11:23:18 +0200 Subject: [PATCH 8/9] Show only one of similar warnings --- libraries/advisory_rules.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/advisory_rules.txt b/libraries/advisory_rules.txt index 2e613b7bb5..042c60f9c0 100644 --- a/libraries/advisory_rules.txt +++ b/libraries/advisory_rules.txt @@ -250,7 +250,7 @@ rule 'Percentage of temp tables on disk' [Created_tmp_tables + Created_tmp_disk_ Increasing {max_heap_table_size} and {tmp_table_size} might help. However some temporary tables are always being written to disk, independent of the value of these variables. To eliminate these you will have to rewrite your queries to avoid those conditions (Within a temporary table: Presence of a BLOB or TEXT column or presence of a column bigger than 512 bytes) as mentioned in the beginning of an Article by the Pythian Group %s% of all temporary tables are being written to disk, this value should be below 25% | round(value,1) -rule 'Temp disk rate' +rule 'Temp disk rate' [!fired('Percentage of temp tables on disk')] Created_tmp_disk_tables / Uptime value * 60 * 60 > 1 Many temporary tables are being written to disk instead of being kept in memory. From 638fb45ff8866cf0d9e91a16ca29b4055eb455c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C4=8Ciha=C5=99?= Date: Thu, 3 May 2012 11:25:19 +0200 Subject: [PATCH 9/9] Wrap long lines --- libraries/advisory_rules.txt | 34 +++++++++++++++++++++++++--------- 1 file changed, 25 insertions(+), 9 deletions(-) diff --git a/libraries/advisory_rules.txt b/libraries/advisory_rules.txt index 042c60f9c0..0408a969ab 100644 --- a/libraries/advisory_rules.txt +++ b/libraries/advisory_rules.txt @@ -1,8 +1,12 @@ # phpMyAdmin Advisory rules file +# # Use only UNIX style newlines -# This file is being parsed by Advisor.class.php, which should handle syntax errors correctly. -# However, PHP Warnings and the like are being consumed by the phpMyAdmin error handler, so those won't show up -# E.g.: Justification line is empty because you used an unescape percent sign, sprintf() returns an empty string and no warning/error is shown +# +# This file is being parsed by Advisor.class.php, which should handle syntax +# errors correctly. However, PHP Warnings and the like are being consumed by +# the phpMyAdmin error handler, so those won't show up E.g.: Justification line +# is empty because you used an unescape percent sign, sprintf() returns an +# empty string and no warning/error is shown # # Rule Syntax: # 'rule' identifier[the name of the rule] eexpr [an optional precondition] @@ -16,14 +20,26 @@ # eexpr: [expr] - expr enclosed in [] # expr: a php code literal with extras: # - variable names are replaced with their respective values -# - fired('name of rule') is replaced with true/false when given rule has been fired. Note however that this is a very simple rules engine. Rules are only checked in sequential order as they are written down here. If given rule has not been checked yet, fired() will always evaluate to false -# - 'value' is replaced with the calculated value. If it is a string, it will be put within single quotes -# - other than that you may use any php function, initialized variable or constant +# - fired('name of rule') is replaced with true/false when given rule has +# been fired. Note however that this is a very simple rules engine. +# Rules are only checked in sequential order as they are written down +# here. If given rule has not been checked yet, fired() will always +# evaluate to false +# - 'value' is replaced with the calculated value. If it is a string, it +# will be put within single quotes +# - other than that you may use any php function, initialized variable or +# constant # # identifier: A string enclosed in single quotes -# string: A quoteless string, may contain HTML. Variable names enclosed in curly braces are replaced with links to directly edit this variable. e.g. {tmp_table_size} -# formatted-string: You may use classic php sprintf() string formatting here, the arguments must be appended after a trailing pipe (|) as mentioned in above syntax -# percent signs (%) are automatically escaped (%%) in the following cases: When followed by a space, dot or comma and at the end of the line) +# string: A quoteless string, may contain HTML. Variable names enclosed in +# curly braces are replaced with links to directly edit this variable. +# e.g. {tmp_table_size} +# formatted-string: You may use classic php sprintf() string formatting here, +# the arguments must be appended after a trailing pipe (|) as +# mentioned in above syntax percent signs (%) are +# automatically escaped (%%) in the following cases: When +# followed by a space, dot or comma and at the end of the +# line) # # Comments start with # #