Tommaso Cucinotta wrote: > AFAICS, the patch detects the "mimetypeapplication" string which appears > in ODG files. However, I'm not an expert and I have no clue about > whether it will misdetect as non-zipped any file (containing that string > by chance) that should actually be considered as zipped (e.g., did you > try to zip an EPS file containing that string :-) ?).
I did not try. There is certainly a chance for misdetection (but not if an EPS file contains that string, rather if the first file in the archive is named like that, and special archive flags are set), but it is very small, and even if a file is misdetected we error on the safe side, because the extension fallback fixes the problem in most cases. The other way round (if a wrong format is detected), can't be fixed at all, so this is much worse. If you are really concerned of this small possibility please don't have a closer look at the detection of the other formats. There are many more cases of possible misdetection ;-) > I'm also a supporter of the idea of using libmagic and getting rid of > all this stuff inside LyX, but that seems also to require a > non-negligible re-engineering work for file formats and converters. For > what it matters, I could claim that the whole of the format detection > and format conversion mechanisms could be confined within an external > command-line tool(s) that might be invoked by LyX as needed. That would be a possibility as well, but it would need refactoring as well. Anyway, I started to use libmagic, and it does not look too bad. In the attached patch I skipped the refactoring part for clarity (although it is not too much). The basic idea is not to parse the text output of libmagic (e.g. "OpenDocument Drawing"), but to use the mime type (e.g. "application/vnd.oasis.opendocument.graphics"). It does not work completely yet, but the basic stuff does work, and as you can see, the amount of needed code is quite small. What do you think? If you like this approach I'll continue (slowly) with completing the work, e.g. prefs2prefs and some more refactoring. Somebody else would need to do the cmake detection. > For now, I can only say that I think bug #7973 had already been fixed by > the zipped=native flag, as I introduced it exactly for properly > supporting ODG to EPS conversions within LyX, so I don't understand why > there was an additional patch for that. As I wrote in trac I was not aware that zipped=native was not in branch, thereore I wrongly believed that it did not fix the problem in this case. > IMO, we should keep only one mechanism for this mis-detection of zipped > files, whatever we think is best. Keeping both seems a mess to me and > makes the code paths harder to follow. Come on, my additional five code lines do not make anything harder to follow. Even without them there are some formats that are correctly detected by guessFormatFromContents(), and others are not. > In the mean-time, I confess I > struggle at understanding what is this zipped format management useful > for, within LyX itself :-) ! For transparent unzipping of some files, e.g. .lyx and some graphics formats. Georg
Index: src/Format.h =================================================================== --- src/Format.h (revision 40599) +++ src/Format.h (working copy) @@ -43,7 +43,7 @@ public: /// Format(std::string const & n, std::string const & e, std::string const & p, std::string const & s, std::string const & v, std::string const & ed, - int); + std::string const & m, int); /// bool dummy() const; /// Is \p ext a valid filename extension for this format? @@ -83,6 +83,10 @@ public: /// void setEditor(std::string const & v) { editor_ = v; } /// + std::string const & mime() const { return mime_; } + /// + void setMime(std::string const & m) { mime_ = m; } + /// bool documentFormat() const { return flags_ & document; } /// bool vectorFormat() const { return flags_ & vector; } @@ -111,6 +115,8 @@ private: std::string viewer_; /// Editor for this format. \sa viewer_. std::string editor_; + /// Full MIME type, e.g. "text/x-tex" + std::string mime_; /// int flags_; }; @@ -159,7 +165,7 @@ public: void add(std::string const & name, std::string const & extensions, std::string const & prettyname, std::string const & shortcut, std::string const & viewer, std::string const & editor, - int flags); + std::string const & mime, int flags); /// void erase(std::string const & name); /// Index: src/frontends/qt4/GuiPrefs.cpp =================================================================== --- src/frontends/qt4/GuiPrefs.cpp (revision 40599) +++ src/frontends/qt4/GuiPrefs.cpp (working copy) @@ -1983,6 +1983,7 @@ void PrefFileformats::on_formatsCB_curre formatED->setText(toqstr(f.name())); copierED->setText(toqstr(form_->movers().command(f.name()))); extensionsED->setText(toqstr(f.extensions())); + mimeED->setText(toqstr(f.mime())); shortcutED->setText( toqstr(l10n_shortcut(f.prettyname(), f.shortcut()))); documentCB->setChecked((f.documentFormat())); @@ -2038,6 +2039,13 @@ void PrefFileformats::on_editorED_textEd } +void PrefFileformats::on_mimeED_textEdited(const QString & s) +{ + currentFormat().setMime(fromqstr(s)); + changed(); +} + + void PrefFileformats::on_shortcutED_textEdited(const QString & s) { string const new_shortcut = fromqstr(s); @@ -2195,7 +2203,7 @@ Format & PrefFileformats::currentFormat( void PrefFileformats::on_formatNewPB_clicked() { - form_->formats().add("", "", "", "", "", "", Format::none); + form_->formats().add("", "", "", "", "", "", "", Format::none); updateView(); formatsCB->setCurrentIndex(0); formatsCB->setFocus(Qt::OtherFocusReason); Index: src/frontends/qt4/ui/PrefFileformatsUi.ui =================================================================== --- src/frontends/qt4/ui/PrefFileformatsUi.ui (revision 40599) +++ src/frontends/qt4/ui/PrefFileformatsUi.ui (working copy) @@ -103,6 +103,19 @@ <item row="5" column="1" colspan="2" > <widget class="QLineEdit" name="extensionsED" /> </item> + <item row="6" column="0" > + <widget class="QLabel" name="mimeLA" > + <property name="text" > + <string>&MIME:</string> + </property> + <property name="buddy" > + <cstring>mimeED</cstring> + </property> + </widget> + </item> + <item row="6" column="1" > + <widget class="QLineEdit" name="mimeED" /> + </item> <item row="7" column="0" > <widget class="QLabel" name="shortcutLA" > <property name="text" > @@ -239,6 +252,7 @@ <tabstop>editorED</tabstop> <tabstop>viewerCO</tabstop> <tabstop>viewerED</tabstop> + <tabstop>mimeED</tabstop> <tabstop>copierED</tabstop> </tabstops> <includes> Index: src/frontends/qt4/GuiPrefs.h =================================================================== --- src/frontends/qt4/GuiPrefs.h (revision 40599) +++ src/frontends/qt4/GuiPrefs.h (working copy) @@ -358,6 +358,7 @@ private Q_SLOTS: void on_extensionsED_textEdited(const QString &); void on_viewerED_textEdited(const QString &); void on_editorED_textEdited(const QString &); + void on_mimeED_textEdited(const QString &); void on_shortcutED_textEdited(const QString &); void on_formatED_editingFinished(); void on_formatED_textChanged(const QString &); Index: src/Format.cpp =================================================================== --- src/Format.cpp (revision 40599) +++ src/Format.cpp (working copy) @@ -37,6 +37,10 @@ #include "support/linkback/LinkBackProxy.h" #endif +#ifdef HAVE_MAGIC_H +#include <magic.h> +#endif + using namespace std; using namespace lyx::support; @@ -78,6 +82,19 @@ private: }; +class FormatMimeEqual : public unary_function<Format, bool> { +public: + FormatMimeEqual(string const & mime) + : mime_(mime) {} + bool operator()(Format const & f) const + { + return f.mime() == mime_ && mime_ != ""; + } +private: + string mime_; +}; + + class FormatPrettyNameEqual : public unary_function<Format, bool> { public: FormatPrettyNameEqual(string const & prettyname) @@ -101,9 +118,9 @@ bool operator<(Format const & a, Format Format::Format(string const & n, string const & e, string const & p, string const & s, string const & v, string const & ed, - int flags) + string const & m, int flags) : name_(n), prettyname_(p), shortcut_(s), viewer_(v), - editor_(ed), flags_(flags) + editor_(ed), mime_(m), flags_(flags) { extension_list_ = getVectorFromString(e, ","); LYXERR(Debug::GRAPHICS, "New Format: n=" << n << ", flags=" << flags); @@ -168,7 +185,38 @@ string Formats::getFormatFromFile(FileNa if (filename.empty()) return string(); +#ifdef HAVE_MAGIC_H + magic_t magic_cookie = magic_open(MAGIC_MIME); + if (magic_cookie) { + LYXERR(Debug::GRAPHICS, "Formats::getFormatFromFile\n\tGot magic cookie"); + string format; + if (magic_load(magic_cookie, NULL) != 0) { + LYXERR(Debug::GRAPHICS, "Formats::getFormatFromFile\n" + << "\tCouldn't load magic database - " + << magic_error(magic_cookie)); + } else { + string mime = magic_file(magic_cookie, + filename.toFilesystemEncoding().c_str()); + mime = token(mime, ';', 0); + LYXERR(Debug::GRAPHICS, "Formats::getFormatFromFile\n" + << "\tGot mime - " + << mime); + // we need our own ps/eps detection + if (mime != "application/postscript") { + Formats::const_iterator cit = + find_if(formatlist.begin(), formatlist.end(), + FormatMimeEqual(mime)); + if (cit != formats.end()) { + LYXERR(Debug::GRAPHICS, "\tgot format from MIME type: " + << mime << " -> " << cit->name()); + format = cit->name(); + } + } + } + magic_close(magic_cookie); + if (!format.empty()) + return format; + } +#endif + string const format = filename.guessFormatFromContents(); string const ext = getExtension(filename.absFileName()); if (FileName::isZippedFileFormat(format) && !ext.empty()) { @@ -291,24 +341,24 @@ void Formats::add(string const & name) { if (!getFormat(name)) add(name, name, name, string(), string(), string(), - Format::document); + string(), Format::document); } void Formats::add(string const & name, string const & extensions, string const & prettyname, string const & shortcut, string const & viewer, string const & editor, - int flags) + string const & mime, int flags) { FormatList::iterator it = find_if(formatlist.begin(), formatlist.end(), FormatNamesEqual(name)); if (it == formatlist.end()) formatlist.push_back(Format(name, extensions, prettyname, - shortcut, viewer, editor, flags)); + shortcut, viewer, editor, mime, flags)); else *it = Format(name, extensions, prettyname, shortcut, viewer, - editor, flags); + editor, mime, flags); } Index: src/LyXRC.cpp =================================================================== --- src/LyXRC.cpp (revision 40599) +++ src/LyXRC.cpp (working copy) @@ -1093,6 +1093,7 @@ LyXRC::ReturnValues LyXRC::read(Lexer & if (lexrc.next(true)) editor = lexrc.getString(); string flags; + string mime; // Hack to ensure compatibility with versions older // than 1.5.0 int le = lexrc.lex(); @@ -1105,6 +1106,16 @@ LyXRC::ReturnValues LyXRC::read(Lexer & // flags. lexrc.pushToken(flags); flags.erase(); + } else { + // same hack for mime + int le = lexrc.lex(); + if (le != Lexer::LEX_FEOF && le != Lexer::LEX_UNDEF) { + mime = lexrc.getString(); + if (le != Lexer::LEX_DATA) { + lexrc.pushToken(mime); + mime.erase(); + } + } } } int flgs = Format::none; @@ -1131,7 +1142,7 @@ LyXRC::ReturnValues LyXRC::read(Lexer & formats.erase(format); } else { formats.add(format, extensions, prettyname, - shortcut, viewer, editor, flgs); + shortcut, viewer, editor, mime, flgs); } break; } Index: configure.ac =================================================================== --- configure.ac (revision 40599) +++ configure.ac (working copy) @@ -115,6 +115,11 @@ AC_CHECK_HEADERS(zlib.h, [AC_CHECK_LIB(z, gzopen, [LIBS="$LIBS -lz"], LYX_LIB_ERROR(libz,zlib))], [LYX_LIB_ERROR(zlib.h,zlib)]) +### check for file magic support +AC_CHECK_HEADERS(magic.h, + [AC_CHECK_LIB(magic, magic_open, [LIBS="$LIBS -lmagic"], LYX_LIB_ERROR(libmagic,libmagic))], + [LYX_LIB_ERROR(magic.h,libmagic)]) + ### check which frontend we want to use Index: lib/configure.py =================================================================== --- lib/configure.py (revision 40599) +++ lib/configure.py (working copy) @@ -483,13 +483,13 @@ def checkFormatEntries(dtl_tools): rc_entry = [r'\Format tgif obj Tgif "" "%%" "%%" "vector"']) # checkViewerEditor('a FIG viewer and editor', ['xfig', 'jfig3-itext.jar', 'jfig3.jar'], - rc_entry = [r'\Format fig fig FIG "" "%%" "%%" "vector"']) + rc_entry = [r'\Format fig fig FIG "" "%%" "%%" "vector" application/x-xfig']) # checkViewerEditor('a Dia viewer and editor', ['dia'], rc_entry = [r'\Format dia dia DIA "" "%%" "%%" "vector,zipped=native"']) # checkViewerEditor('an OpenOffice drawing viewer and editor', ['libreoffice', 'lodraw', 'ooffice', 'oodraw', 'soffice'], - rc_entry = [r'\Format odg "odg, sxd" "OpenOffice drawing" "" "%%" "%%" "vector,zipped=native"']) + rc_entry = [r'\Format odg "odg, sxd" "OpenOffice drawing" "" "%%" "%%" "vector,zipped=native" application/vnd.oasis.opendocument.graphics']) # checkViewerEditor('a Grace viewer and editor', ['xmgrace'], rc_entry = [r'\Format agr agr Grace "" "%%" "%%" "vector"']) @@ -498,40 +498,40 @@ def checkFormatEntries(dtl_tools): rc_entry = [r'\Format fen fen FEN "" "%%" "%%" ""']) # checkViewerEditor('a SVG viewer and editor', ['inkscape'], - rc_entry = [r'\Format svg svg SVG "" "%%" "%%" "vector"']) + rc_entry = [r'\Format svg svg SVG "" "%%" "%%" "vector" image/svg+xml']) # path, iv = checkViewerNoRC('a raster image viewer', ['xv', 'kview', 'gimp-remote', 'gimp'], - rc_entry = [r'''\Format bmp bmp BMP "" "%s" "%s" "" -\Format gif gif GIF "" "%s" "%s" "" -\Format jpg "jpg, jpeg" JPEG "" "%s" "%s" "" -\Format pbm pbm PBM "" "%s" "%s" "" -\Format pgm pgm PGM "" "%s" "%s" "" -\Format png png PNG "" "%s" "%s" "" -\Format ppm ppm PPM "" "%s" "%s" "" -\Format tiff tif TIFF "" "%s" "%s" "" -\Format xbm xbm XBM "" "%s" "%s" "" -\Format xpm xpm XPM "" "%s" "%s" ""''']) + rc_entry = [r'''\Format bmp bmp BMP "" "%s" "%s" "" image/x-bmp +\Format gif gif GIF "" "%s" "%s" "" image/gif +\Format jpg "jpg, jpeg" JPEG "" "%s" "%s" "" image/jpeg +\Format pbm pbm PBM "" "%s" "%s" "" image/x-portable-bitmap +\Format pgm pgm PGM "" "%s" "%s" "" image/x-portable-graymap +\Format png png PNG "" "%s" "%s" "" image/x-png +\Format ppm ppm PPM "" "%s" "%s" "" image/x-portable-pixmap +\Format tiff tif TIFF "" "%s" "%s" "" image/tiff +\Format xbm xbm XBM "" "%s" "%s" "" image/x-xbitmap +\Format xpm xpm XPM "" "%s" "%s" "" image/x-xpixmap''']) path, ie = checkEditorNoRC('a raster image editor', ['gimp-remote', 'gimp'], - rc_entry = [r'''\Format bmp bmp BMP "" "%s" "%s" "" -\Format gif gif GIF "" "%s" "%s" "" -\Format jpg "jpg, jpeg" JPEG "" "%s" "%s" "" -\Format pbm pbm PBM "" "%s" "%s" "" -\Format pgm pgm PGM "" "%s" "%s" "" -\Format png png PNG "" "%s" "%s" "" -\Format ppm ppm PPM "" "%s" "%s" "" -\Format tiff tif TIFF "" "%s" "%s" "" -\Format xbm xbm XBM "" "%s" "%s" "" -\Format xpm xpm XPM "" "%s" "%s" ""''']) - addToRC(r'''\Format bmp bmp BMP "" "%s" "%s" "" -\Format gif gif GIF "" "%s" "%s" "" -\Format jpg "jpg, jpeg" JPEG "" "%s" "%s" "" -\Format pbm pbm PBM "" "%s" "%s" "" -\Format pgm pgm PGM "" "%s" "%s" "" -\Format png png PNG "" "%s" "%s" "" -\Format ppm ppm PPM "" "%s" "%s" "" -\Format tiff tif TIFF "" "%s" "%s" "" -\Format xbm xbm XBM "" "%s" "%s" "" -\Format xpm xpm XPM "" "%s" "%s" ""''' % \ + rc_entry = [r'''\Format bmp bmp BMP "" "%s" "%s" "" image/x-bmp +\Format gif gif GIF "" "%s" "%s" "" image/gif +\Format jpg "jpg, jpeg" JPEG "" "%s" "%s" "" image/jpeg +\Format pbm pbm PBM "" "%s" "%s" "" image/x-portable-bitmap +\Format pgm pgm PGM "" "%s" "%s" "" image/x-portable-graymap +\Format png png PNG "" "%s" "%s" "" image/x-png +\Format ppm ppm PPM "" "%s" "%s" "" image/x-portable-pixmap +\Format tiff tif TIFF "" "%s" "%s" "" image/tiff +\Format xbm xbm XBM "" "%s" "%s" "" image/x-xbitmap +\Format xpm xpm XPM "" "%s" "%s" "" image/x-xpixmap''']) + addToRC(r'''\Format bmp bmp BMP "" "%s" "%s" "" image/x-bmp +\Format gif gif GIF "" "%s" "%s" "" image/gif +\Format jpg "jpg, jpeg" JPEG "" "%s" "%s" "" image/jpeg +\Format pbm pbm PBM "" "%s" "%s" "" image/x-portable-bitmap +\Format pgm pgm PGM "" "%s" "%s" "" image/x-portable-graymap +\Format png png PNG "" "%s" "%s" "" image/x-png +\Format ppm ppm PPM "" "%s" "%s" "" image/x-portable-pixmap +\Format tiff tif TIFF "" "%s" "%s" "" image/tiff +\Format xbm xbm XBM "" "%s" "%s" "" image/x-xbitmap +\Format xpm xpm XPM "" "%s" "%s" "" image/x-xpixmap''' % \ (iv, ie, iv, ie, iv, ie, iv, ie, iv, ie, iv, ie, iv, ie, iv, ie, iv, ie, iv, ie) ) # checkViewerEditor('a text editor', ['xemacs', 'gvim', 'kedit', 'kwrite', 'kate', \ @@ -550,11 +550,11 @@ def checkFormatEntries(dtl_tools): \Format r R "R/S code" "" "" "%%" "document,menu=export" \Format lilypond ly "LilyPond music" "" "" "%%" "vector" \Format lilypond-book lytex "LilyPond book (LaTeX)" "" "" "%%" "document,menu=export" -\Format latex tex "LaTeX (plain)" L "" "%%" "document,menu=export" +\Format latex tex "LaTeX (plain)" L "" "%%" "document,menu=export" text/x-tex \Format luatex tex "LaTeX (LuaTeX)" "" "" "%%" "document,menu=export" \Format pdflatex tex "LaTeX (pdflatex)" "" "" "%%" "document,menu=export" \Format xetex tex "LaTeX (XeTeX)" "" "" "%%" "document,menu=export" -\Format text txt "Plain text" a "" "%%" "document,menu=export" +\Format text txt "Plain text" a "" "%%" "document,menu=export" text/plain \Format text2 txt "Plain text (pstotext)" "" "" "%%" "document" \Format text3 txt "Plain text (ps2ascii)" "" "" "%%" "document" \Format text4 txt "Plain text (catdvi)" "" "" "%%" "document" @@ -579,40 +579,40 @@ def checkFormatEntries(dtl_tools): # rc_entry = [ r'\ps_command "%%"' ]) checkViewer('a Postscript previewer', ['kghostview', 'okular', 'evince', 'gv', 'ghostview -swap'], rc_entry = [r'''\Format eps eps EPS "" "%%" "" "vector" -\Format ps ps Postscript t "%%" "" "document,vector,menu=export"''']) +\Format ps ps Postscript t "%%" "" "document,vector,menu=export" application/postscript''']) # for xdg-open issues look here: http://www.mail-archive.com/lyx-devel@lists.lyx.org/msg151818.html checkViewer('a PDF previewer', ['kpdf', 'okular', 'evince', 'kghostview', 'xpdf', 'acrobat', 'acroread', \ 'gv', 'ghostview'], - rc_entry = [r'''\Format pdf pdf "PDF (ps2pdf)" P "%%" "" "document,vector,menu=export" + rc_entry = [r'''\Format pdf pdf "PDF (ps2pdf)" P "%%" "" "document,vector,menu=export" application/pdf \Format pdf2 pdf "PDF (pdflatex)" F "%%" "" "document,vector,menu=export" \Format pdf3 pdf "PDF (dvipdfm)" m "%%" "" "document,vector,menu=export" \Format pdf4 pdf "PDF (XeTeX)" X "%%" "" "document,vector,menu=export" \Format pdf5 pdf "PDF (LuaTeX)" u "%%" "" "document,vector,menu=export"''']) # checkViewer('a DVI previewer', ['xdvi', 'kdvi', 'okular', 'yap', 'dviout -Set=!m'], - rc_entry = [r'''\Format dvi dvi DVI D "%%" "" "document,vector,menu=export" + rc_entry = [r'''\Format dvi dvi DVI D "%%" "" "document,vector,menu=export" application/x-dvi \Format dvi3 dvi "DVI (LuaTeX)" V "%%" "" "document,vector,menu=export"''']) if dtl_tools: # Windows only: DraftDVI addToRC(r'\Format dvi2 dvi DraftDVI "" "" "" "vector"') # checkViewer('an HTML previewer', ['firefox', 'mozilla file://$$p$$i', 'netscape'], - rc_entry = [r'\Format html "html, htm" HTML H "%%" "" "document,menu=export"']) + rc_entry = [r'\Format html "html, htm" HTML H "%%" "" "document,menu=export" text/html']) # checkViewerEditor('Noteedit', ['noteedit'], rc_entry = [r'\Format noteedit not Noteedit "" "%%" "%%" "vector"']) # checkViewerEditor('an OpenDocument/OpenOffice viewer', ['lwriter', 'swriter', 'oowriter', 'abiword'], - rc_entry = [r'''\Format odt odt OpenDocument "" "%%" "%%" "document,vector,menu=export" -\Format sxw sxw "OpenOffice.Org (sxw)" "" "" "" "document,vector"''']) + rc_entry = [r'''\Format odt odt OpenDocument "" "%%" "%%" "document,vector,menu=export" application/vnd.oasis.opendocument.text +\Format sxw sxw "OpenOffice.Org (sxw)" "" "" "" "document,vector" application/vnd.sun.xml.writer''']) # checkViewerEditor('a Rich Text and Word viewer', ['lwriter', 'swriter', 'oowriter', 'abiword'], - rc_entry = [r'''\Format rtf rtf "Rich Text Format" "" "%%" "%%" "document,vector,menu=export" -\Format word doc "MS Word" W "%%" "%%" "document,vector,menu=export"''']) + rc_entry = [r'''\Format rtf rtf "Rich Text Format" "" "%%" "%%" "document,vector,menu=export" application/rtf +\Format word doc "MS Word" W "%%" "%%" "document,vector,menu=export" application/msword''']) # # entries that do not need checkProg addToRC(r'''\Format date "" "date command" "" "" "" "" -\Format csv csv "Table (CSV)" "" "" "" "document" +\Format csv csv "Table (CSV)" "" "" "" "document" text/csv \Format fax "" Fax "" "" "" "document" \Format lyx lyx LyX "" "" "" "" \Format lyx13x 13.lyx "LyX 1.3.x" "" "" "" "document"