maximago [maximago]

Which should the file to review?

I guess you mean the file attached in comment #1. Thanks for the answer.

1.     '#options' => array(
         silverlightvideo_autoload_true => t('TRUE'),
         silverlightvideo_autoload_false => t('FALSE'),
       ),
   

   The array indexes are not strings, or numbers.

2.     '#title' => t('Display Timecode'),
   	'#default_value' => variable_get('displaytimecode', displaytimecode_false),
   

   Strings used in user interface should have the first word in capital case, and the other words in lower case (with the exception of proper nouns, adjectives derived from proper nouns, and acronyms).
   The Drupal variable used doesn't respect the namespace.

3.   $form['silverlightvideo']['general']['startmuted'] = array(
       '#type' => 'radios',
       '#title' => t('Start muted'),
   	'#default_value' => variable_get('startmuted', startmuted_false),
       '#options' => array(
         silverlightvideo_startmuted_true => t('TRUE'),
         silverlightvideo_startmuted_false => t('FALSE'),
       ),
     );
   

   Why isn't the code using a checkbox?

4. // Some constant definitions to make later code clearer
   define('silverlightvideo_DO_NOT_DISPLAY', 3);
   define('silverlightvideo_DEFAULT_HTML_ALT', 'You are missing a video that should appear here! Perhaps your browser cannot display it, or maybe it did not initialise correctly.');
   define('silverlightvideo_DEFAULT_WEIGHT', -0.1);
   define('silverlightvideo_DEFAULT_IMPORT_STATUS', 0);
   define('silverlightvideo_DEFAULT_PATH', 'silverlightvideos');
   define('silverlightvideo_DEFAULT_EXTENSIONS', 'wmv');
   

   See the Drupal coding standards to understand how a module code should be written.

5. /**
    * Implementation of hook_perm
    */
   function silverlightvideo_perm() {
     return array(
       t('administer silverlightvideos'),
       t('create silverlightvideos'),
       t('edit own silverlightvideos'),
       t('edit any silverlightvideo'),
       t('delete own silverlightvideos'),
       t('delete any silverlightvideo'),
       t('import video'),
     );
   }
   

   The hook should pass the permission strings without to translate them.

6.   $items['admin/settings/silverlightvideo'] = array(
       'title' => t('silverlightvideo'),
       'description' => t('Set various silverlightvideo node  default options.'),
   

   Menu titles, and descriptions are not passed to t().

7.     drupal_set_message('New node');
   

   The string is not translated; then, the message seems a debug message that should be removed.

8.     '#description' => $node->silverlightvideo['fid'] ? t('<font color ="red"><b>Current file is %filename. Click "Browse..." to upload a different file.</b></font>', array('%filename' => basename($node->silverlightvideo['filepath']))) : t('Click "Browse..." to select a file to upload.'),
   

   The attribute color is deprecated in favor of CSS styles.

9.     '#type' => 'fieldset',
   	'#title' => t('Advance settings'),
   

   It should be Advanced settings.

10. function silverlightvideo_view($node, $teaser = "FALSE", $page = 0) {
    

    $teaser is not a string.

11.     if ($node->type != 'silverlightvideo') {
          $error = t('Macro tried to load node @nid which is not a silverlightvideo node .', array('@nid' => $args['nid']));
        }
      }
    
      // If the initial phase failed display a message and log in watchdog for admin
      if ($error) {
        //drupal_set_message($error, 'error');
        watchdog('silverlightvideo', $error, NULL, WATCHDOG_ERROR);
    

    The second argument of watchdog() is not correct.

12.     case 'description':
          return t('Add silverlight from a silverlightvideo node  to your posts using a silverlightvideo node  macro.');
          break;
          
        case 'process':
          foreach(silverlightvideo_get_macros($text) as $unexpanded_macro => $macro) {
            $replace = silverlightvideo_content($macro, $format);
            $search = '@(<p>)?'.preg_quote($unexpanded_macro).'(</p>)?@';
            $text = preg_replace($search, $replace, $text);
          }
          return $text;
          break;
    

    It doesn't seem that the code verify if the user has access to the silverlightvideo node, before to return any data about that node. This is a security issue.

13.   $silverlightvideo_path = file_create_path(variable_get('silverlightvideo_default_path', silverlightvideo_DEFAULT_PATH));
      $silverlightvideo_temp_path = $silverlightvideo_path .'/temp';
    
      // Check if directories exist, create them if not
      file_check_directory($silverlightvideo_path, FILE_CREATE_DIRECTORY, 'silverlightvideo_default_path');
    

    The code doesn't seem correct; first it uses variable_get('silverlightvideo_default_path', silverlightvideo_DEFAULT_PATH), and then 'silverlightvideo_default_path'.

14. // Helper function
    function _silverlightvideo_directorytoarray($directory, $recursive) {
      $array_items = array();
      if ($handle = opendir($directory)) {
        while (false !== ($file = readdir($handle))) {
          if ($file != "." && $file != "..") {
            if (is_dir($directory. "/" . $file)) {
              if ($recursive) {
                $array_items = array_merge($array_items, _silverlightvideo_directorytoarray($directory. "/" . $file, $recursive));
              }
              $file = $directory . "/" . $file;
              $array_items[] = preg_replace("/\/\//si", "/", $file);
            }
            else {
              $file = $directory . "/" . $file;
              $array_items[] = preg_replace("/\/\//si", "/", $file);
            }
          }
        }
        closedir($handle);
      }
      return $array_items;
    }
    

    Why isn't the code using a Drupal function with the same purpose?

15.   $form['files'] = array('#prefix' => '<ul>', '#suffix' => '</ul>', '#tree' => TRUE);
    
      // array_filter in the call returned only elements with TRUE values
      foreach ($files as $file) {
        $form['files'][$file] = array('#type' => 'hidden', '#value' => $file, '#prefix' => '<li>', '#suffix' => check_plain(str_replace(file_create_path(variable_get('silverlightvideo_default_path', silverlightvideo_DEFAULT_PATH)).'/', '', $file)) ."</li>\n");
      }
    

    Hidden items are not shown in the HTML page; the code is building a list of items that are not visible.

16.   $node = new stdClass();
      $file = new stdClass();
      
      // Omissis.
      
      $node->silverlightvideo['height'] = $info['height'];
      $node->silverlightvideo['width'] = $info['width'];
      $file->filemime = $info['mime_type'];
    
      // Set a flag to tell silverlightvideo_insert we are adding files via import - currently suppresses file validation
      $node->silverlightvideo['import'] = TRUE;
    
      // Prepare the file object
      $file->uid = $user->uid;
      $file->filename = basename($file_to_import);
      $file->filepath = $file_to_import;
      $file->status = FILE_STATUS_PERMANENT;
      $file->timestamp = time();
      $file->filesize = filesize(realpath($file_to_import));
    
      // If we didn't get mime type from earlier attempt we will need to try and guess it from the extension
      if (!$file->filemime) {
        if (preg_match('@wmv$@i', $file->filename, $matches)) {
        $file->filemime = 'Audio-/X-ms-wmv';
        }
      }
    
      // Write file to the database
      drupal_write_record('files', $file);
    
      // Add fid to the silverlightvideo object
      $node->silverlightvideo['fid'] = db_last_insert_id('files', 'fid');
    
      // Save the node
      node_save($node);
    

    Before to save a node that has been built in memory, the code should allow to third-party modules to add their own data.

17.   else {
        drupal_set_message(t('Failed to import ').$file_to_import, 'warning');
      }
    

    Rather than concatenating two strings, the code should use a placeholder.

18.     'description' => t('Stores details associated with each silverlight file, such as height, width and display mode.'),
    

    Schema descriptions should not be passed to t() anymore. See system_schema() for an example of what done by Drupal core code.

  $form['silverlightvideo']['general'] = array(
    '#type' => 'fieldset',
	'#title' => t('General settings'),
	'#collapsible' => TRUE,
    '#collapsed' => TRUE,
  );

  $form['silverlightvideo_updated']['general'] = array(
    '#type' => 'hidden',
    '#value' => time(),
  );

  $form['silverlightvideo']['general']['autoload'] = array(
    '#type' => 'radios',
    '#title' => t('Auto load'),
    '#default_value' => variable_get('autoload', autoload_true),
    '#options' => array(
      t('TRUE'),t('FALSE'),
    ),
  );

See the Drupal coding standards to understand how a module code should be written.
In particular, see how the code should be formatted, and the part about the namespace respect.
Also, I don't see the definition of the constant autoload_true in that file, and the file is not including another file.

Hello I am new to the drupal community and have just started learning to use it. I would like to integrate Silverlight into my site and this is the only thing I can find on the website regarding Silverlight. However, since I cannot find this work under the Modules section of this website, it is unclear to me if this is something that can be used now? I see that I could download the attached .zip file (silverlightvideo.zip) but again I am concerned about the status of said code etc.
Thanks,
Dex

The code contains code lines as variable_get(framerate, ''), which are not correct for two reasons (I have already reported why such lines are not correct).

There have not been replies in the last week. I am marking this application as won't fix.