maximago [maximago]

please don´t use this version

The first file :)

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

   The array indexes are not strings, or numbers.

2. <?php
    '#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. <?php
  $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. <?php
// 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. <?php
/**
* 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. <?php
  $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. <?php
    drupal_set_message('New node');
?>

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

8. <?php
    '#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. <?php
    '#type' => 'fieldset',
    '#title' => t('Advance settings'),
?>

   It should be Advanced settings.

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

    $teaser is not a string.

11. <?php
    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. <?php
    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. <?php
  $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. <?php
// 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. <?php
  $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. <?php
  $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. <?php
  else {
    drupal_set_message(t('Failed to import ').$file_to_import, 'warning');
  }
?>

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

18. <?php
    '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.

Hello,
In the Attachment is a newer version.
The following Points were fixed:
1.2, 5, 6, 7, 8, 9, 10, 11, 12, 17, 18

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).