silverstripe / silverstripe-assets

Silverstripe Assets component
BSD 3-Clause "New" or "Revised" License
9 stars 66 forks source link

Support additional file types #202

Closed jinjie closed 5 years ago

jinjie commented 5 years ago

get_file_type() in SilverStripe\Assets\File have limited file types supported.

    /**
     * Get descriptive type of file based on filename
     *
     * @param string $filename
     * @return string Description of file
     */
    public static function get_file_type($filename)
    {
        $types = array(
            'gif' => _t(__CLASS__.'.GifType', 'GIF image - good for diagrams'),
            'jpg' => _t(__CLASS__.'.JpgType', 'JPEG image - good for photos'),
            'jpeg' => _t(__CLASS__.'.JpgType', 'JPEG image - good for photos'),
            'png' => _t(__CLASS__.'.PngType', 'PNG image - good general-purpose format'),
            'ico' => _t(__CLASS__.'.IcoType', 'Icon image'),
            'tiff' => _t(__CLASS__.'.TiffType', 'Tagged image format'),
            'doc' => _t(__CLASS__.'.DocType', 'Word document'),
            'xls' => _t(__CLASS__.'.XlsType', 'Excel spreadsheet'),
            'zip' => _t(__CLASS__.'.ZipType', 'ZIP compressed file'),
            'gz' => _t(__CLASS__.'.GzType', 'GZIP compressed file'),
            'dmg' => _t(__CLASS__.'.DmgType', 'Apple disk image'),
            'pdf' => _t(__CLASS__.'.PdfType', 'Adobe Acrobat PDF file'),
            'mp3' => _t(__CLASS__.'.Mp3Type', 'MP3 audio file'),
            'wav' => _t(__CLASS__.'.WavType', 'WAV audio file'),
            'avi' => _t(__CLASS__.'.AviType', 'AVI video file'),
            'mpg' => _t(__CLASS__.'.MpgType', 'MPEG video file'),
            'mpeg' => _t(__CLASS__.'.MpgType', 'MPEG video file'),
            'js' => _t(__CLASS__.'.JsType', 'Javascript file'),
            'css' => _t(__CLASS__.'.CssType', 'CSS file'),
            'html' => _t(__CLASS__.'.HtmlType', 'HTML file'),
            'htm' => _t(__CLASS__.'.HtmlType', 'HTML file')
        );

        // Get extension
        $extension = strtolower(self::get_file_extension($filename));
        return isset($types[$extension]) ? $types[$extension] : 'unknown';
    }

When I try to get a descriptive file type in my template for a uploaded file (in this case, a docx) $File.FileType. It returns "unknown" file type.

I am thinking how this can better improved rather than maintaining a list of array value map of extensions to the descriptive name. Maybe a configurable yml file?

ScopeyNZ commented 5 years ago

Making this configuration makes sense to me. Would you like to raise a PR against the 1 branch?

jinjie commented 5 years ago

@ScopeyNZ Should I add a new configuration that stores the additional file types? So users can easily add new file extensions by yml. Something like

$types = array_merge($types, self::get_additional_file_types());

I guess doing this change will also break translation of the file types?

robbieaverill commented 5 years ago

@jinjie I realise you've already made a pull request, but to address your question I think that making the whole list part of configuration makes more sense. Things like this should usually be configurable by default when they're created. This part isn't, which may be a relic of past versions of SilverStripe. Your pull request is the right approach in my opinion =)

maxime-rainville commented 5 years ago

Docs still needs to be merged

https://github.com/silverstripe/silverstripe-framework/pull/8709

ScopeyNZ commented 5 years ago

Docs are merged :)