View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0011163 | Scribus | Internal | public | 2012-11-11 12:04 | 2016-04-05 19:59 |
Reporter | ale | Assigned To | |||
Priority | normal | Severity | feature | Reproducibility | N/A |
Status | new | Resolution | open | ||
Product Version | 1.5.0svn | ||||
Summary | 0011163: [PATCH] Simplify and extend QString FormatsManager::extensionListForFormat() | ||||
Description | For the Epub plugin I need a list of extensions attached to a file format. Instead of splitting the result of the current extensionListForFormat(), I've reimplemented it to both return a QStringList or a QString, depending on the number of parameters. I've taken the chance to massively simplify the existing method and use the tools already provided by QMap and QString. I don't think that there should be any performance concerns that would justify to reimplement value(), join() and trimmed() (if ever the current implementation was faster than the one of mine). As noted in the comment, in the future, I think that the method returning a QString should be renamed and only be used for the "*.jpg *.jpeg" type of lists. This would further simplify it and it's really not that hard for the caller to do extensionListForFormat(1).join("|"). IMO it's even much more readable then the current way (setting the second parameter to 1 or any other number...) | ||||
Tags | epub | ||||
Patch | Yes | ||||
|
util_formats.patch (2,838 bytes)
Index: scribus/util_formats.cpp =================================================================== --- scribus/util_formats.cpp (revision 17837) +++ scribus/util_formats.cpp (working copy) @@ -172,44 +172,29 @@ return a + b + ";;" +c; } +QStringList FormatsManager::extensionListForFormat(int type) +{ + QStringList result; + if ((JPEG & type) && !m_supportedImageFormats.contains(QByteArray("jpg"))) + return result; + if ((GIF & type) && !m_supportedImageFormats.contains(QByteArray("gif"))) + return result; + if (m_fmts.contains(type)) + result = m_fmts.value(type); + return result; +} + QString FormatsManager::extensionListForFormat(int type, int listType) { - QString nameMatch; - QString separator(listType==0 ? " *." : "|"); - QMapIterator<int, QStringList> it(m_fmts); - bool first=true; - int n=0; - while (it.hasNext()) - { - it.next(); - if (type & it.key()) - { - //Just in case the Qt used doesn't support jpeg or gif - if ((JPEG & it.key()) && !m_supportedImageFormats.contains(QByteArray("jpg"))) - continue; - if ((GIF & it.key()) && !m_supportedImageFormats.contains(QByteArray("gif"))) - continue; - if (first) - first=false; - QStringListIterator itSL(it.value()); - while (itSL.hasNext()) - { - if (listType==0) - nameMatch += separator; - nameMatch += itSL.next(); - if (listType==1 && itSL.hasNext()) - nameMatch += separator; - } - } - ++n; - if (listType==1 && it.hasNext() && nameMatch.length()>0 && !nameMatch.endsWith(separator)) - nameMatch += separator; - } - if (listType==0 && nameMatch.startsWith(" ")) - nameMatch.remove(0,1); - if (listType==1 && nameMatch.endsWith("|")) - nameMatch.chop(1); - return nameMatch; + // TODO: This method should probably be renamed and only used to return the "*." type of lists + // For the "piped ones" it's probably easier and faster to directly call the method above and do a join + // (ale/20121111) + QString result; + QString separator(listType == 0 ? " *." : "|"); + result = extensionListForFormat(type).join(separator).trimmed(); + if (listType == 0) + result = "*." + result; + return result; } void FormatsManager::fileTypeStrings(int type, QString& formatList, QString& formatText, QString& formatAll, bool lowerCaseOnly) Index: scribus/util_formats.h =================================================================== --- scribus/util_formats.h (revision 17837) +++ scribus/util_formats.h (working copy) @@ -121,6 +121,8 @@ //! Returns in the form of "EPS (*.eps *.EPS *.epsf *.EPSF *.epsi *.EPSI)" QString extensionsForFormat(int type); + //! Returns a list of extensions matching the type + QStringList extensionListForFormat(int type); //! Returns in the form of "*.eps *.epsf *.epsi" or "eps|epsf|epsi" QString extensionListForFormat(int type, int listType); |
|
i finally got the right indenting... |
|
The patch won't work in the case where a set of various format is passed as input parameters. See for eg paintmanager.cpp line 1049 : extensionListForFormat(FormatsManager::EPS|FormatsManager::PS|FormatsManager::AI, 0) So the while loop cannot be avoided... |
|
looks like a good way to make simple things complex and complex things simple... in my eyes, the method should be simplified and eventually a second method should be added for the more complex ones (using the first method and then merging the results). |
Date Modified | Username | Field | Change |
---|---|---|---|
2012-11-11 12:04 | ale | New Issue | |
2012-11-11 12:04 | ale | File Added: util_formats.patch | |
2012-11-11 12:08 | ale | File Deleted: util_formats.patch | |
2012-11-11 12:08 | ale | File Added: util_formats.patch | |
2012-11-11 12:10 | ale | File Deleted: util_formats.patch | |
2012-11-11 12:10 | ale | File Added: util_formats.patch | |
2012-11-11 14:06 | ale | Note Added: 0029189 | |
2012-11-25 12:35 | jghali | Note Added: 0029269 | |
2012-11-27 08:29 | ale | Note Added: 0029282 | |
2014-10-24 23:00 | Kunda | Patch | => Yes |
2016-04-05 19:59 | Kunda | Tag Attached: epub | |
2016-04-05 19:59 | Kunda | Category | General => Internal |