* move common file-specific functionality out of IDF_Views_Source into new

IDF_FileUtil and change all occurrences accordingly
* cache /etc/mime.types (or whatever is configured) per request in a static
variable in IDF_FileUtil
* always link directly to the download of attached files in the issues view
and place an additional "view" link only for those attachments which we
recognize as text with our weak criteria (closes issue 575)

src/IDF/Diff.php

@@ -169,8 +169,8 @@ class IDF_Diff
         $out = '';
         foreach ($this->files as $filename=>$file) {
             $pretty = '';
-            $fileinfo = IDF_Views_Source::getMimeType($filename);
+            $fileinfo = IDF_FileUtil::getMimeType($filename);
-            if (IDF_Views_Source::isSupportedExtension($fileinfo[2])) {
+            if (IDF_FileUtil::isSupportedExtension($fileinfo[2])) {
                 $pretty = ' prettyprint';
             }
             $out .= "\n".'<table class="diff" summary="">'."\n";
@@ -350,9 +350,9 @@ class IDF_Diff
 
     public function renderCompared($chunks, $filename)
     {
-        $fileinfo = IDF_Views_Source::getMimeType($filename);
+        $fileinfo = IDF_FileUtil::getMimeType($filename);
         $pretty = '';
-        if (IDF_Views_Source::isSupportedExtension($fileinfo[2])) {
+        if (IDF_FileUtil::isSupportedExtension($fileinfo[2])) {
             $pretty = ' prettyprint';
         }
         $out = '';

src/IDF/FileUtil.php

@@ -0,0 +1,165 @@
+<?php
+/* -*- tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
+/*
+# ***** BEGIN LICENSE BLOCK *****
+# This file is part of InDefero, an open source project management application.
+# Copyright (C) 2010 Céondo Ltd and contributors.
+#
+# InDefero is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# InDefero is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+#
+# ***** END LICENSE BLOCK ***** */
+
+/**
+ * File utilities.
+ *
+ */
+class IDF_FileUtil
+{
+    /**
+     * Extension supported by the syntax highlighter.
+     */
+    public static $supportedExtenstions = array(
+              'ascx', 'ashx', 'asmx', 'aspx', 'browser', 'bsh', 'c', 'cl', 'cc',
+              'config', 'cpp', 'cs', 'csh', 'csproj', 'css', 'cv', 'cyc', 'el', 'fs',
+              'h', 'hh', 'hpp', 'hs', 'html', 'html', 'java', 'js', 'lisp', 'master',
+              'pas', 'perl', 'php', 'pl', 'pm', 'py', 'rb', 'scm', 'sh', 'sitemap',
+              'skin', 'sln', 'svc', 'vala', 'vb', 'vbproj', 'vbs', 'wsdl', 'xhtml',
+              'xml', 'xsd', 'xsl', 'xslt');
+
+    /**
+     * Test if an extension is supported by the syntax highlighter.
+     *
+     * @param string The extension to test
+     * @return bool
+     */
+    public static function isSupportedExtension($extension)
+    {
+        return in_array($extension, self::$supportedExtenstions);
+    }
+
+    /**
+     * Returns a HTML snippet with a line-by-line pre-rendered table
+     * for the given source content
+     *
+     * @param array file information as returned by getMimeType or getMimeTypeFromContent
+     * @param string the content of the file
+     * @return string
+     */
+    public static function highLight($fileinfo, $content)
+    {
+        $pretty = '';
+        if (self::isSupportedExtension($fileinfo[2])) {
+            $pretty = ' prettyprint';
+        }
+        $table = array();
+        $i = 1;
+        foreach (preg_split("/\015\012|\015|\012/", $content) as $line) {
+            $table[] = '<tr class="c-line"><td class="code-lc" id="L'.$i.'"><a href="#L'.$i.'">'.$i.'</a></td>'
+                .'<td class="code mono'.$pretty.'">'.IDF_Diff::padLine(Pluf_esc($line)).'</td></tr>';
+            $i++;
+        }
+        return Pluf_Template::markSafe(implode("\n", $table));
+    }
+
+    /**
+     * Find the mime type of a file.
+     *
+     * Use /etc/mime.types to find the type.
+     *
+     * @param string Filename/Filepath
+     * @param array  Mime type found or 'application/octet-stream', basename, extension
+     */
+    public static function getMimeType($file)
+    {
+        static $mimes = null;
+        if ($mimes == null) {
+            $mimes = array();
+            $src = Pluf::f('idf_mimetypes_db', '/etc/mime.types');
+            $filecontent = @file_get_contents($src);
+            if ($filecontent !== false) {
+                $mimes = preg_split("/\015\012|\015|\012/", $filecontent);
+            }
+        }
+
+        $info = pathinfo($file);
+        if (isset($info['extension'])) {
+            foreach ($mimes as $mime) {
+                if ('#' != substr($mime, 0, 1)) {
+                    $elts = preg_split('/ |\t/', $mime, -1, PREG_SPLIT_NO_EMPTY);
+                    if (in_array($info['extension'], $elts)) {
+                        return array($elts[0], $info['basename'], $info['extension']);
+                    }
+                }
+            }
+        } else {
+            // we consider that if no extension and base name is all
+            // uppercase, then we have a text file.
+            if ($info['basename'] == strtoupper($info['basename'])) {
+                return array('text/plain', $info['basename'], 'txt');
+            }
+            $info['extension'] = 'bin';
+        }
+        return array('application/octet-stream', $info['basename'], $info['extension']);
+    }
+
+    /**
+     * Find the mime type of a file using the fileinfo class.
+     *
+     * @param string Filename/Filepath
+     * @param string File content
+     * @return array Mime type found or 'application/octet-stream', basename, extension
+     */
+    public static function getMimeTypeFromContent($file, $filedata)
+    {
+        $info = pathinfo($file);
+        $res = array('application/octet-stream',
+                     $info['basename'],
+                     isset($info['extension']) ? $info['extension'] : 'bin');
+        if (function_exists('finfo_open')) {
+            $finfo = finfo_open(FILEINFO_MIME);
+            $mime = finfo_buffer($finfo, $filedata);
+            finfo_close($finfo);
+            if ($mime) {
+                $res[0] = $mime;
+            }
+            if (!isset($info['extension']) && $mime) {
+                $res[2] = (0 === strpos($mime, 'text/')) ? 'txt' : 'bin';
+            } elseif (!isset($info['extension'])) {
+                $res[2] = 'bin';
+            }
+        }
+        return $res;
+    }
+
+    /**
+     * Find if a given mime type is a text file.
+     * This uses the output of the self::getMimeType function.
+     *
+     * @param array (Mime type, file name, extension)
+     * @return bool Is text
+     */
+    public static function isText($fileinfo)
+    {
+        if (0 === strpos($fileinfo[0], 'text/')) {
+            return true;
+        }
+        $ext = 'mdtext php-dist h gitignore diff patch';
+        $extra_ext = trim(Pluf::f('idf_extra_text_ext', ''));
+        if (!empty($extra_ext))
+            $ext .= ' ' . $extra_ext;
+        $ext = array_merge(self::$supportedExtenstions, explode(' ' , $ext));
+        return (in_array($fileinfo[2], $ext));
+    }
+}

src/IDF/IssueFile.php

@@ -128,4 +128,10 @@ class IDF_IssueFile extends Pluf_Model
     {
         @unlink(Pluf::f('upload_issue_path').'/'.$this->attachment);
     }
+
+    function isText()
+    {
+        $info = IDF_FileUtil::getMimeType($this->filename);
+        return IDF_FileUtil::isText($info);
+    }
 }

src/IDF/Template/Markdown.php

@@ -93,7 +93,7 @@ class IDF_Template_Markdown extends Pluf_Template_Tag
         $info = pathinfo($m[1]);
         $fileinfo = array($res->headers['Content-Type'], $m[1], 
                           isset($info['extension']) ? $info['extension'] : 'bin');
-        if (!IDF_Views_Source::isText($fileinfo)) {
+        if (!IDF_FileUtil::isText($fileinfo)) {
             return $m[0];
         }
         return $res->content;

src/IDF/Tests/TestFileUtil.php

@@ -0,0 +1,47 @@
+<?php
+/* -*- tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
+/*
+# ***** BEGIN LICENSE BLOCK *****
+# This file is part of InDefero, an open source project management application.
+# Copyright (C) 2008 Céondo Ltd and contributors.
+#
+# InDefero is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# InDefero is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+#
+# ***** END LICENSE BLOCK ***** */
+
+/**
+ * Tests some of the FileUtils
+ */
+class IDF_Tests_TestFileUtil extends UnitTestCase 
+{
+ 
+    public function __construct() 
+    {
+        parent::__construct('Test the file utils.');
+    }
+
+    public function testGetMimeType()
+    {
+        $files = array(
+                       'whatever.php' => 'application/x-httpd-php',
+                       'whatever.pht' => 'application/x-httpd-php',
+                       'README' => 'text/plain',
+                       );
+        foreach ($files as $file => $mime) {
+            $m = IDF_Views_Source::getMimeType($file);
+            $this->assertEqual($mime, $m[0]);
+        }
+    }
+}

src/IDF/Tests/TestSource.php

@@ -32,19 +32,6 @@ class IDF_Tests_TestSource extends UnitTestCase
         parent::__construct('Test the source class.');
     }
 
-    public function testGetMimeType()
-    {
-        $files = array(
-                       'whatever.php' => 'application/x-httpd-php',
-                       'whatever.pht' => 'application/x-httpd-php',
-                       'README' => 'text/plain',
-                       );
-        foreach ($files as $file => $mime) {
-            $m = IDF_Views_Source::getMimeType($file);
-            $this->assertEqual($mime, $m[0]);
-        }
-    }
-
     public function testRegexCommit()
     {
         $regex = '#^/p/([\-\w]+)/source/tree/([^\/]+)/(.*)$#';

src/IDF/Views/Issue.php

@@ -306,7 +306,7 @@ class IDF_Views_Issue
         $prj = $request->project;
         $attach = Pluf_Shortcuts_GetObjectOr404('IDF_IssueFile', $match[2]);
         $prj->inOr404($attach->get_comment()->get_issue());
-        $info = IDF_Views_Source::getMimeType($attach->filename);
+        $info = IDF_FileUtil::getMimeType($attach->filename);
         $mime = 'application/octet-stream';
         if (strpos($info[0], 'image/') === 0) {
             $mime = $info[0];
@@ -330,13 +330,13 @@ class IDF_Views_Issue
         $prj->inOr404($attach->get_comment()->get_issue());
         // If one cannot see the attachement, redirect to the
         // getAttachment view.
-        $info = IDF_Views_Source::getMimeType($attach->filename);
+        $info = IDF_FileUtil::getMimeType($attach->filename);
-        if (!IDF_Views_Source::isText($info)) {
+        if (!IDF_FileUtil::isText($info)) {
             return $this->getAttachment($request, $match);
         }
         // Now we want to look at the file but with links back to the
         // issue.
-        $file = IDF_Views_Source::highLight($info, 
+        $file = IDF_FileUtil::highLight($info,
                                         file_get_contents(Pluf::f('upload_issue_path').'/'.$attach->attachment));
         $title = sprintf(__('View %s'), $attach->filename);
         return Pluf_Shortcuts_RenderToResponse('idf/issues/attachment.html',

src/IDF/Views/Source.php

@@ -31,17 +31,6 @@ Pluf::loadFunction('Pluf_Shortcuts_GetFormForModel');
  */
 class IDF_Views_Source
 {
-    /**
-     * Extension supported by the syntax highlighter.
-     */
-    public static $supportedExtenstions = array(
-              'ascx', 'ashx', 'asmx', 'aspx', 'browser', 'bsh', 'c', 'cl', 'cc',
-              'config', 'cpp', 'cs', 'csh', 'csproj', 'css', 'cv', 'cyc', 'el', 'fs',
-              'h', 'hh', 'hpp', 'hs', 'html', 'html', 'java', 'js', 'lisp', 'master',
-              'pas', 'perl', 'php', 'pl', 'pm', 'py', 'rb', 'scm', 'sh', 'sitemap', 
-              'skin', 'sln', 'svc', 'vala', 'vb', 'vbproj', 'vbs', 'wsdl', 'xhtml',
-              'xml', 'xsd', 'xsl', 'xslt');
-
     /**
      * Display help on how to checkout etc.
      */
@@ -211,7 +200,7 @@ class IDF_Views_Source
         if ($request_file_info->type != 'tree') {
             $info = self::getRequestedFileMimeType($request_file_info,
                                                    $commit, $scm);
-            if (!self::isText($info)) {
+            if (!IDF_FileUtil::isText($info)) {
                 $rep = new Pluf_HTTP_Response($scm->getFile($request_file_info),
                                               $info[0]);
                 $rep->headers['Content-Disposition'] = 'attachment; filename="'.$info[1].'"';
@@ -373,7 +362,7 @@ class IDF_Views_Source
         $previous = substr($request_file, 0, -strlen($l.' '));
         $scmConf = $request->conf->getVal('scm', 'git');
         $props = $scm->getProperties($commit, $request_file);
-        $content = self::highLight($extra['mime'], $scm->getFile($request_file_info));
+        $content = IDF_FileUtil::highLight($extra['mime'], $scm->getFile($request_file_info));
         return Pluf_Shortcuts_RenderToResponse('idf/source/'.$scmConf.'/file.html',
                                                array(
                                                      'page_title' => $page_title,
@@ -451,123 +440,14 @@ class IDF_Views_Source
      */
     public static function getRequestedFileMimeType($file_info, $commit, $scm)
     {
-        $mime = self::getMimeType($file_info->file);
+        $mime = IDF_FileUtil::getMimeType($file_info->file);
         if ('application/octet-stream' != $mime[0]) {
             return $mime;
         }
-        return self::getMimeTypeFromContent($file_info->file,
+        return IDF_FileUtil::getMimeTypeFromContent($file_info->file,
                                                     $scm->getFile($file_info));
     }
 
-     /**
-      * Find the mime type of a file using the fileinfo class.
-      *
-      * @param string Filename/Filepath
-      * @param string File content
-      * @return array Mime type found or 'application/octet-stream', basename, extension
-      */
-    public static function getMimeTypeFromContent($file, $filedata)
-    {
-        $info = pathinfo($file);
-        $res = array('application/octet-stream',
-                     $info['basename'],
-                     isset($info['extension']) ? $info['extension'] : 'bin');
-        if (function_exists('finfo_open')) {
-            $finfo = finfo_open(FILEINFO_MIME);
-            $mime = finfo_buffer($finfo, $filedata);
-            finfo_close($finfo);
-            if ($mime) {
-                $res[0] = $mime;
-            }
-            if (!isset($info['extension']) && $mime) {
-                $res[2] = (0 === strpos($mime, 'text/')) ? 'txt' : 'bin';
-            } elseif (!isset($info['extension'])) {
-                $res[2] = 'bin';
-            }
-        }
-        return $res;
-    }
-
-    /**
-     * Find the mime type of a file.
-     *
-     * Use /etc/mime.types to find the type.
-     *
-     * @param string Filename/Filepath
-     * @param array  Mime type found or 'application/octet-stream', basename, extension
-     */
-    public static function getMimeType($file)
-    {
-        $src= Pluf::f('idf_mimetypes_db', '/etc/mime.types');
-        $mimes = preg_split("/\015\012|\015|\012/", file_get_contents($src));
-        $info = pathinfo($file);
-        if (isset($info['extension'])) {
-            foreach ($mimes as $mime) {
-                if ('#' != substr($mime, 0, 1)) {
-                    $elts = preg_split('/ |\t/', $mime, -1, PREG_SPLIT_NO_EMPTY);
-                    if (in_array($info['extension'], $elts)) {
-                        return array($elts[0], $info['basename'], $info['extension']);
-                    }
-                }
-            }
-        } else {
-            // we consider that if no extension and base name is all
-            // uppercase, then we have a text file.
-            if ($info['basename'] == strtoupper($info['basename'])) {
-                return array('text/plain', $info['basename'], 'txt');
-            }
-            $info['extension'] = 'bin';
-        }
-        return array('application/octet-stream', $info['basename'], $info['extension']);
-    }
-
-    /**
-     * Find if a given mime type is a text file.
-     * This uses the output of the self::getMimeType function.
-     *
-     * @param array (Mime type, file name, extension)
-     * @return bool Is text
-     */
-    public static function isText($fileinfo)
-    {
-        if (0 === strpos($fileinfo[0], 'text/')) {
-            return true;
-        }
-        $ext = 'mdtext php-dist h gitignore diff patch';
-        $extra_ext = trim(Pluf::f('idf_extra_text_ext', ''));
-        if (!empty($extra_ext))
-            $ext .= ' ' . $extra_ext;
-        $ext = array_merge(self::$supportedExtenstions, explode(' ' , $ext));
-        return (in_array($fileinfo[2], $ext));
-    }
-
-    public static function highLight($fileinfo, $content)
-    {
-        $pretty = '';
-        if (self::isSupportedExtension($fileinfo[2])) {
-            $pretty = ' prettyprint';
-        }
-        $table = array();
-        $i = 1;
-        foreach (preg_split("/\015\012|\015|\012/", $content) as $line) {
-            $table[] = '<tr class="c-line"><td class="code-lc" id="L'.$i.'"><a href="#L'.$i.'">'.$i.'</a></td>'
-                .'<td class="code mono'.$pretty.'">'.IDF_Diff::padLine(Pluf_esc($line)).'</td></tr>';
-            $i++;
-        }
-        return Pluf_Template::markSafe(implode("\n", $table));
-    }
-
-    /**
-     * Test if an extension is supported by the syntax highlighter.
-     *
-     * @param string The extension to test
-     * @return bool
-     */
-    public static function isSupportedExtension($extension)
-    {
-        return in_array($extension, self::$supportedExtenstions);
-    }
-
     /**
      * Get the scm type for page title
      *

src/IDF/templates/idf/issues/view.html

@@ -20,8 +20,7 @@
 {if $attachments.count() > 0}
 <hr align="left" class="attach" />
 <ul>
-{foreach $attachments as $a}<li><a href="{url 'IDF_Views_Issue::viewAttachment', array($project.shortname, $a.id, $a.filename)}">{$a.filename}</a> - {$a.filesize|size}</li>{/foreach}
+{foreach $attachments as $a}<li><a href="{url 'IDF_Views_Issue::getAttachment', array($project.shortname, $a.id, $a.filename)}" title="{trans 'download'}">{$a.filename}</a> - {$a.filesize|size}{if $a.isText()} - <a href="{url 'IDF_Views_Issue::viewAttachment', array($project.shortname, $a.id, $a.filename)}">{trans 'view'}</a>{/if}</li>{/foreach}</ul>{/if}
-</ul>{/if}
 {if $i> 0 and $c.changedIssue()}
 <div class="issue-changes">
 {foreach $c.changes as $w => $v}