plegall / Piwigo-write_metadata

3 stars 4 forks source link

Request: Add error checking #4

Open vovan69 opened 8 years ago

vovan69 commented 8 years ago

I've tried this plugin and it works in most of the cases pretty fine. However there's absolutely no error checking for execution of exiftool command: in case if the exiftool fails I'm getting nice green success message "Metadata written into file". I think it would be nice to implement at least some checks or print at least output in case of errors (use exiftool option -q, so it prints only errors). I'm not a PHP programmer, but bellow is working example. I'd appreciate any comments/suggestions, etc. Besides a basic error checking there's some other adjustments, which I personally found useful.

--- main.inc.php.orig   2016-05-11 17:26:18.110528591 +0200
+++ main.inc.php        2016-05-13 15:50:13.393285216 +0200
@@ -54,9 +54,9 @@
   if (isset($page['page']) and 'photo' == $page['page'] and isset($_GET['write_metadata']))
   {
     check_input_parameter('image_id', $_GET, false, PATTERN_ID);
-    wm_write_metadata($_GET['image_id']);
+    list ( $rc, $output ) = wm_write_metadata($_GET['image_id']);

-    $_SESSION['page_infos'][] = l10n('Metadata written into file');
+    $_SESSION['page_infos'][] = l10n('Metadata written into file').' ('.join('; ', $output).' )';
     redirect(get_root_url().'admin.php?page=photo-'.$_GET['image_id'].'-properties');
   }
 }
@@ -136,11 +136,13 @@
   $tags = wm_prepare_string($row['tags'], 64);

   $command = isset($conf['exiftool_path']) ? $conf['exiftool_path'] : 'exiftool';
+  $command.= ' -use mwg -overwrite_original -charset UTF8 -charset IPTC=UTF8';

   if (strlen($name) > 0)
   {
     #2#105 in iptcparse($imginfo['APP13'])
     $command.= ' -IPTC:Headline="'.$name.'"';
+    $command.= ' -XMP:Title="'.$name.'"';

     #2#005 in iptcparse($imginfo['APP13'])
     $command.= ' -IPTC:ObjectName="'.wm_cutString($name, 64).'"';
@@ -177,13 +179,14 @@
   }

   $command.= ' "'.$row['path'].'"';
+  $command.= ' 2>&1';
   // echo $command;

-  $exec_return = exec($command, $output);
+  $exec_return = exec($command, $output, $rc);
   // echo '$exec_return = '.$exec_return.'<br>';
   // echo '<pre>'; print_r($output); echo '</pre>';

-  return true;
+  return array ( $rc, $output );
 }
plegall commented 8 years ago

@vovan69 I implemented you system for output (I have added the -q option) and I show the errors as errors when they occur, instead of "info". Please tell me what you think.

Please open another issue for the XMP:Title and others for the options you suggest (mwg, overwrite, utf8...)

vovan69 commented 8 years ago

Thanks for your time! I've tried two options for checking if metadata has been written successfully:

  1. relay on the exit code of exiftool
  2. if something printed in the (error)output (2>&1) while using '-q' option According to my tests the option 2 is the one I can relay on. In this case the "$rc" can be omitted, just to keep the code clean (in your commit the $rc var isn't used anyway). One more point: could you explain why you use '$page['errors'] = array_merge($page['errors'], $output);' instead of, for instance, '$_SESSION['page_errors'][] = l10n('Error writing metadata').' ('.join('; ', $output).' )';'?