Finish Running Through Coder

Please commit. It's a lot easier to contribute to a module when it follows the code standards.

Second pass through the code:
- fixed comments for hook implementations
- removed double brackets from simple strings
- refactored block hooks' implementations according drupal standards
- removed debug code and commented code
- another small fixes according standards

Still needs work:
- refactoring repeating code
- implement theme function using hook_theme (now these are just functions)
- fix output generation to use theme functions or render arrays
- there is some strange page callback which does nothing: contentanalysis_util()

a lot of trailing whitespaces

+++ b/contentanalysis.admin.incundefined
@@ -37,10 +36,10 @@
-    '#type' => 'radios',    
+    '#type' => 'radios', ¶

trailing whitespaces

+++ b/contentanalysis.admin.incundefined
@@ -37,10 +36,10 @@
-    '#default_value' => variable_get('contentanalysis_node_report_display', 'both'),     
+    '#default_value' => variable_get('contentanalysis_node_report_display', 'both'), ¶
     '#description' => t('Select the style of report you would like when launching from the node edit form. Dialog is the standard modal report. Inline inserts recommendations directly into the node edit form.')     

+++ b/contentanalysis.admin.incundefined
@@ -63,17 +62,11 @@
+  drupal_set_title(t('Analyzer settings'), PASS_THROUGH); ¶
   $form = contentanalysis_admin_analyzer_settings_element();

trailing whitespaces

+++ b/contentanalysis.jsundefined
@@ -1,356 +1,360 @@
-		  //alert('data.nid ' + data.nid)
...
+      var tabs = $("#contentanalysis-report-tabs-" + e[3]);

You have to remove debug data

+++ b/contentanalysis.jsundefined
@@ -1,356 +1,360 @@
+
+      //alert('data.nid ' + data.nid)

You have to remove debug data

+++ b/contentanalysis.jsundefined
@@ -1,356 +1,360 @@
+            $('#analysis-modal .progress').remove();
+            //Drupal.behaviors.contentanalysisui();

You have to remove debug data

+++ b/contentanalysis.jsundefined
@@ -1,356 +1,360 @@
+            if (data.inputs['action'] == 'refresh') {
+              //if($('.contentanalysis_section_analysis').length > 0)

You have to remove debug data

+++ b/contentanalysis.jsundefined
@@ -1,356 +1,360 @@
+                  if (meta_title.indexOf("[node:title]") == -1) {
+                    //show_title = false;

You have to remove debug data

+++ b/contentanalysis.jsundefined
@@ -1,356 +1,360 @@
+          }
+          // call any modules post analysis hooks      ¶

trailing whitespace

+++ b/contentanalysis.moduleundefined
@@ -496,115 +501,130 @@ function contentanalysis_analyze_js() {
+    'aid' => NULL,
+    // analysis id
+    'form_id' => NULL,
+    // id of form that submitted the content
+    'source' => NULL,
+    // source of request
+    'nid' => NULL,
+    // the Drupal node id
+    'path' => NULL,
+    // the Drupal path, after base_path, for a Drupal page
+    'url' => NULL,
+    // full url including http://
+    'page' => NULL,
+    // the contents of a page if a full page is to be analyzed, e.g. by using a URL to fetch a page
+    'title' => NULL,
+    // a drupal page title, e.g. a node title or header for other types of Drupal pages
+    'node_body' => NULL,
+    // content of the node body field
+    'body' => NULL,
+    // the main body of text to be analyzed. for a node it is the node body, plus title or any cck fields. For a page it is all content between the body tags
+    'body_notags' => NULL,
+    // the body field with all stripped of all HTML tags
+    'page_title' => NULL,
+    // the title found between the title tags in the header section of a HTMLdoc.
+    'meta_title' => NULL,
+    // the meta title found in the ehader section of a HTML doc
+    'meta_keywords' => NULL,
+    // the meta keywords found in the header section of a HTML doc.
+    'meta_description' => NULL,
+    // the meta keywords found in the header section of a HTML doc.
     'analyzers' => array(),

looks like this part of documentation better move to phpdoc block

+++ b/contentanalysis.moduleundefined
@@ -612,47 +632,47 @@ function contentanalysis_get_default_context() {
+  }
   elseif ($context['nid'] > 0) { // analysis by nid
     return contentanalysis_parse_context_node_load($context, $analysis);
-  } 
+  }
   elseif ($context['body']) { // analysis by direct text input
     $context['body_notags'] = strip_tags($context['body']);
-  } 
+  }
   elseif ($context['page']) { // analysis by url

this part of docs have no Drupal styling

+++ b/includes/seo_friend.incundefined
@@ -25,25 +24,25 @@ function contentanalysis_seo_friend_report() {
-	FROM {contentanalysis_status}

+++ b/includes/seo_friend.incundefined
@@ -80,40 +79,40 @@ function contentanalysis_seo_friend_report() {
+        $value = ($value) ? $value : t('NA');
+        $class = 'status';    ¶

trailing whitespace

+++ b/includes/seo_friend.incundefined
@@ -80,40 +79,40 @@ function contentanalysis_seo_friend_report() {
-        $class = 'ok';		
+        $class = 'ok';        ¶

trailing whitespace

+++ b/includes/seo_friend.incundefined
@@ -80,40 +79,40 @@ function contentanalysis_seo_friend_report() {
+        $class = 'error';
+      }    ¶
+      $data[] = array(
+        'data' => $value,
+        'class' => $class,
+      );
+    }    ¶
+    $data[] = l(t('Edit'), 'node/' . $r->nid . '/edit');

trailing whitespace

@podarok, I have fixed all your comments

changing status to needs review

oops, incorrect diff

+++ b/contentanalysis.cssundefined
@@ -412,42 +423,42 @@ a.contentanalysis-button {
+  text-shadow: 0 1px 0 #FFFFFF;
+  -moz-border-radius: 11px 11px 11px 11px;
+  -moz-box-sizing: content-box;

good to see here
-webkit- (Chrome, Safari)
-o- (Opera)
support CSS

+++ b/contentanalysis.jsundefined
@@ -1,356 +1,356 @@
+      $$.contentanalysis_show_analyzer_tab(contentanalysisPrevAnalyzerTab);
+      //contentanalysis_show_report_tab(contentanalysisPrevReportTab);

debug functions

+++ b/contentanalysis.jsundefined
@@ -1,356 +1,356 @@
+      var tabs = $("#contentanalysis-report-tabs-" + e[3]);
+      //tabs.css('border','2px solid red');

debug functions

+++ b/contentanalysis.jsundefined
@@ -1,356 +1,356 @@
+      var result_id = sec_id.replace('-' + e[4], '')
+      //$('#' + result_id).css({'top': top, 'left': left}); ¶
+      $('#' + result_id).css('top', top);
+      //$('#' + result_id).css('border', '2px solid green'); ¶
+      $('#' + sec_id).show();
+      //alert("pos.left="+pos.left+",pos.top="+pos.top+",offset.left="+offset.left+",offset.top="+offset.top);

trailing whitespaces

+++ b/contentanalysis.jsundefined
@@ -1,356 +1,356 @@
+      Drupal.settings.contentanalysis.display_inline = 1;
+      //$('.contentanalysis-refresh-link-' + analyzer).replaceWith('<span class="throbber">Loading...</span>');
+      $('.contentanalysis-refresh-link-' + analyzer).replaceWith('<div class="ahah-progress ahah-progress-throbber"><div class="throbber">&nbsp;</div></div>');

debug code

+++ b/contentanalysis.moduleundefined
@@ -87,33 +84,31 @@ function contentanalysis_menu() {
-//dsm($ret);
+  $ret = metatag_metatags_view('global', array());
+  $ret = metatag_metatags_view('node:article', $metatags, $options);
 

looks like a weird code - $ret used twice without any logic

+++ b/contentanalysis.moduleundefined
@@ -496,115 +501,137 @@ function contentanalysis_analyze_js() {
   }
   foreach ($analysis AS $anid => $analysisi) {
-    if (substr($anid, 0, 1)=='#' || !is_array($analysisi)) { 
-      continue; 
+    if (substr($anid, 0, 1) == '#' || !is_array($analysisi)) {
+      continue;
     }
-    if((isset($analysisi['#status']) && ($analysisi['#status'] != 'status')) || ($analysisi['#score'])) {
-		  $statusi = array(
-	      'error' => 0,
-			  'warning' => 1,
-			  'complete' => 2,
-		  );
-		  $fields = array(
-		    'status' => $analysisi['#status'],
-		    'statusi' => $statusi[$analysisi['#status']],
-		  );
-		  if($analysisi['#score']) {
-		    $fields['score'] = $analysisi['#score'];
-		  }
-		  $keys = array(
+    if ((isset($analysisi['#status']) && ($analysisi['#status'] != 'status')) || ($analysisi['#score'])) {
+      $statusi = array(
+        'error' => 0,
+        'warning' => 1,
+        'complete' => 2,
+      );
+      $fields = array(
+        'status' => $analysisi['#status'],
+        'statusi' => $statusi[$analysisi['#status']],
+      );
+      if ($analysisi['#score']) {

variable naming hell
analysis
analysis
analisisi
status
statusi

brrrrrrrrrrrr

- fixed issues pointed by @podarok in comment #8
- removed contentanalysis.admin.css since it was empty
- page callback, which just was rendering form, replaced with proper implementation of menu router.
- removed menu route contentanalysis/util which did nothing

updating status

updated patch and interdiff with patch in comment #7

+++ b/sites/all/modules/contentanalysis/contentanalysis.cssundefined
@@ -66,6 +74,7 @@
+  -moz-box-shadow: -1em 1em 1em rgba(0, 0, 0, 0.5);

You need generic box-shadow too

FYI http://www.w3.org/TR/css3-background/#box-shadow

+++ b/sites/all/modules/contentanalysis/contentanalysis.cssundefined
@@ -425,7 +396,11 @@ a.contentanalysis-button {
   -moz-border-radius: 11px 11px 11px 11px;
+  -webkit-border-radius: 11px 11px 11px 11px;
...
   -moz-box-sizing: content-box;
+  -webkit-box-sizing: content-box;

the same - generic border radius

http://www.w3.org/TR/css3-background/#the-border-radius

all other - looks good!

@podarok, thanks. I have fixed those issues and attached final patch.

+++ b/sites/all/modules/contentanalysis/contentanalysis.admin.incundefined
@@ -90,21 +83,20 @@ function contentanalysis_admin_analyzer_settings_element($mode = 'global') {
+        '#title' => $def['title'],
+        //'#description' => t('Check the below boxes to display widgets in node links by content types.'),

debug code

all other looks good

and please...
do interdiffs

thanks, fixed

#16 the end
Thanks!!!

original code:

-  border-top-left-radius: 0.5em;
-  border-top-right-radius: 0.5em;

your replacement

  border-radius: 0.5em 0;

should be

  border-radius: 0.5em 0.5em 0 0;

@gumanist, thanks, fixed

#19 thanks!
Looks nice and really clean

not really :)

not equal replacement

@@ -160,14 +159,10 @@ div.page-analysis div.analysis-results .analyzer-analysis  {
 }
  
 .contentanalysis-report-tab {
-  -webkit-border-radius-topleft: 0.5em;
-  -webkit-border-radius-topright: 0.5em;
-  -moz-border-radius-topleft: 0.5em;
-  -moz-border-radius-topright: 0.5em;
-  border-radius-topleft: 0.5em;
-  border-radius-topright: 0.5em;    
-  /* display: inline; */
-  float: left; 
+  border-radius: 0.5em 0;
+  -webkit-border-radius: 0.5em 0;
+  -moz-border-radius: 0.5em 0;
+  float: left;

it is a good practice to put standards version of style under the vendor prefixed. for example:

+  border-radius: 11px 11px 11px 11px;
+  -moz-border-radius: 11px 11px 11px 11px;
+  -webkit-border-radius: 11px 11px 11px 11px;
+  -o-border-radius: 11px 11px 11px 11px;

should be

+  -moz-border-radius: 11px 11px 11px 11px;
+  -webkit-border-radius: 11px 11px 11px 11px;
+  -o-border-radius: 11px 11px 11px 11px;
+  border-radius: 11px 11px 11px 11px;

use same syntax for all url(). now patch contains few implementations:

+  background: url("images/white-grad-active.png") repeat-x scroll left top #EEEEEE;
+  background-image: url('icons/warning.png');
+  background: #fff url(images/arrow-active.png) no-repeat scroll right center;

same is for colors naming, parameters order

"Implementation of" is deprecated

+   * Implementation of hook_contentanalysis_data()

@gumanist, thanks, I've fixed it, please review

updating patch with some improvements of the CSS

#24 nice

What is the status of this being reviewed and committed to the dev?

Patch will be committed in the next release. Thanks!

adding note about block.