Skip to content

Commit 13a8aa6

Browse files
committed
* Fixed issue #98: Show only one log warning message about missing icon for a file extension.
1 parent 3ecaf2e commit 13a8aa6

File tree

4 files changed

+140
-1
lines changed

4 files changed

+140
-1
lines changed

CHANGES

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ Changes for 0.7.0
99
* Fixed issue #92: New property for detecting file type based on content.
1010
* Fixed issue #96: Problems with Files > 2Gb
1111
* Fixed issue #97: Files and folders together do not start MENU.
12+
* Fixed issue #98: Show only one log warning message about missing icon for a file extension.
1213

1314

1415
Changes for 0.6.1

include/shellanything/Icon.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727

2828
#include <string>
2929
#include <vector>
30+
#include <set>
3031

3132
namespace shellanything
3233
{
@@ -93,6 +94,8 @@ namespace shellanything
9394
void SetIndex(const int & index);
9495

9596
private:
97+
typedef std::set<std::string /*file extension*/> FileExtensionSet;
98+
static FileExtensionSet mUnresolvedFileExtensions;
9699
std::string mFileExtension;
97100
std::string mPath;
98101
int mIndex;

src/Icon.cpp

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
*********************************************************************************/
2424

2525
#include "shellanything/Icon.h"
26+
#include "shellanything/Context.h"
2627
#include "PropertyManager.h"
2728
#include "Win32Registry.h"
2829

@@ -31,8 +32,12 @@
3132
#include <glog/logging.h>
3233
#pragma warning( pop )
3334

35+
#include "rapidassist/strings.h"
36+
3437
namespace shellanything
3538
{
39+
Icon::FileExtensionSet Icon::mUnresolvedFileExtensions;
40+
3641
Icon::Icon() :
3742
mIndex(Icon::INVALID_ICON_INDEX)
3843
{
@@ -74,6 +79,16 @@ namespace shellanything
7479
std::string file_extension = pmgr.Expand(mFileExtension);
7580
if (!file_extension.empty())
7681
{
82+
//check for multiple values. keep the first value, forget about other selected file extensions.
83+
const std::string separator = pmgr.GetProperty(Context::MULTI_SELECTION_SEPARATOR_PROPERTY_NAME);
84+
if (file_extension.find(separator) != std::string::npos)
85+
{
86+
//multiple values detected.
87+
ra::strings::StringVector extension_list = ra::strings::Split(file_extension, separator.c_str());
88+
if (!extension_list.empty())
89+
file_extension = extension_list[0];
90+
}
91+
7792
//try to find the path to the icon module for the given file extension.
7893
Win32Registry::REGISTRY_ICON resolved_icon = Win32Registry::GetFileTypeIcon(file_extension.c_str());
7994
if (!resolved_icon.path.empty() && resolved_icon.index != Win32Registry::INVALID_ICON_INDEX)
@@ -90,10 +105,19 @@ namespace shellanything
90105
//failed to find a valid icon.
91106
//using the default "unknown" icon
92107
Win32Registry::REGISTRY_ICON unknown_file_icon = Win32Registry::GetUnknownFileTypeIcon();
93-
LOG(WARNING) << "Failed to find icon for file extension '" << file_extension << "'. Resolving icon with default icon for unknown file type '" << unknown_file_icon.path << "' with index '" << unknown_file_icon.index << "'";
94108
mPath = unknown_file_icon.path;
95109
mIndex = unknown_file_icon.index;
96110
mFileExtension = "";
111+
112+
//show the message only once in logs
113+
const bool is_already_in_log = mUnresolvedFileExtensions.find(file_extension) != mUnresolvedFileExtensions.end();
114+
if (!is_already_in_log)
115+
{
116+
LOG(WARNING) << "Failed to find icon for file extension '" << file_extension << "'. Resolving icon with default icon for unknown file type '" << unknown_file_icon.path << "' with index '" << unknown_file_icon.index << "'";
117+
118+
//remember this failure.
119+
mUnresolvedFileExtensions.insert(file_extension);
120+
}
97121
}
98122
}
99123
}

test/TestIcon.cpp

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,12 +24,36 @@
2424

2525
#include "TestIcon.h"
2626
#include "shellanything/Icon.h"
27+
#include "shellanything/Context.h"
28+
#include "PropertyManager.h"
29+
#include "GlogUtils.h"
30+
#include "rapidassist/filesystem.h"
2731

2832
namespace shellanything { namespace test
2933
{
34+
int CountString(const std::string & text, const std::string & token)
35+
{
36+
int count = 0;
37+
size_t offset = 0;
38+
size_t pos = 0;
39+
do
40+
{
41+
pos = text.find(token, offset);
42+
if (pos != std::string::npos)
43+
{
44+
count++;
45+
offset = pos + 1;
46+
}
47+
} while (pos != std::string::npos);
48+
return count;
49+
}
50+
3051
//--------------------------------------------------------------------------------------------------
3152
void TestIcon::SetUp()
3253
{
54+
//delete all previous log to be able to identify the current log file
55+
static const int MAX_AGE_SECONDS = -1;
56+
DeletePreviousLogs(MAX_AGE_SECONDS);
3357
}
3458
//--------------------------------------------------------------------------------------------------
3559
void TestIcon::TearDown()
@@ -49,6 +73,93 @@ namespace shellanything { namespace test
4973
ASSERT_NE( Icon::INVALID_ICON_INDEX, icon.GetIndex() );
5074
}
5175
//--------------------------------------------------------------------------------------------------
76+
TEST_F(TestIcon, testResolveMultipleFileExtensionIcon) // issue #98
77+
{
78+
// Solve icon for txt files
79+
Icon icon_txt;
80+
icon_txt.SetFileExtension("txt");
81+
icon_txt.ResolveFileExtensionIcon();
82+
ASSERT_TRUE(icon_txt.GetFileExtension().empty());
83+
ASSERT_FALSE(icon_txt.GetPath().empty());
84+
ASSERT_NE(Icon::INVALID_ICON_INDEX, icon_txt.GetIndex());
85+
86+
//get the separator for multiple values
87+
shellanything::PropertyManager & pmgr = shellanything::PropertyManager::GetInstance();
88+
const std::string separator = pmgr.GetProperty(Context::MULTI_SELECTION_SEPARATOR_PROPERTY_NAME);
89+
90+
//build a list of multiple file extensions
91+
std::string file_extensions;
92+
file_extensions += "txt";
93+
file_extensions += separator;
94+
file_extensions += "xml";
95+
file_extensions += separator;
96+
file_extensions += "jpg";
97+
file_extensions += separator;
98+
file_extensions += "mp3";
99+
file_extensions += separator;
100+
file_extensions += "exe";
101+
file_extensions += separator;
102+
file_extensions += "dll";
103+
file_extensions += separator;
104+
file_extensions += "mp4";
105+
106+
Icon icon;
107+
icon.SetFileExtension(file_extensions);
108+
109+
//act
110+
icon.ResolveFileExtensionIcon();
111+
112+
ASSERT_TRUE(icon.GetFileExtension().empty());
113+
ASSERT_EQ(icon_txt.GetPath(), icon.GetPath());
114+
ASSERT_EQ(icon_txt.GetIndex(), icon.GetIndex());
115+
}
116+
//--------------------------------------------------------------------------------------------------
117+
TEST_F(TestIcon, testMultipleResolveFailures) // issue #98
118+
{
119+
static const std::string UNKNOWN_FILE_EXTENSION = "123456789";
120+
121+
Icon icon;
122+
icon.SetFileExtension(UNKNOWN_FILE_EXTENSION);
123+
124+
//act
125+
icon.ResolveFileExtensionIcon();
126+
127+
//assert to unknown file type
128+
static const std::string UNKNOWN_ICON_FILE_PATH = "C:\\Windows\\system32\\imageres.dll";
129+
static const int UNKNOWN_ICON_INDEX = 2;
130+
ASSERT_TRUE(icon.GetFileExtension().empty());
131+
ASSERT_EQ(UNKNOWN_ICON_FILE_PATH, icon.GetPath());
132+
ASSERT_EQ(UNKNOWN_ICON_INDEX, icon.GetIndex());
133+
134+
//act (issue #98)
135+
for (int i = 0; i < 500; i++)
136+
{
137+
//this should create multiple log entries if the feature is not properly implemented.
138+
Icon tmp_icon;
139+
tmp_icon.SetFileExtension(UNKNOWN_FILE_EXTENSION);
140+
tmp_icon.ResolveFileExtensionIcon();
141+
}
142+
143+
//get log files content
144+
std::string log_dir = GetLogDirectory();
145+
ra::strings::StringVector files;
146+
bool find_success = ra::filesystem::FindFiles(files, log_dir.c_str());
147+
ASSERT_TRUE(find_success);
148+
for (size_t i = 0; i < files.size(); i++)
149+
{
150+
const std::string & path = files[i];
151+
if (IsLogFile(path))
152+
{
153+
std::string content;
154+
bool read = ra::filesystem::ReadTextFile(path, content);
155+
ASSERT_TRUE(read) << "Failed to read log file: " << path;
156+
157+
int count = CountString(content, UNKNOWN_FILE_EXTENSION);
158+
ASSERT_LE(count, 1) << "Log file '" << path << "' contains " << count << " references to '" << UNKNOWN_FILE_EXTENSION << "'.";
159+
}
160+
}
161+
}
162+
//--------------------------------------------------------------------------------------------------
52163

53164
} //namespace test
54165
} //namespace shellanything

0 commit comments

Comments
 (0)