Issue #276 - eliminate CString from GetDataPath

Issue #276 - eliminate CString from GetDataPath

Change the return type and arguments of GetDataPath from CString to
std::wstring. Removed CString from its implementation.

Moved GetDataPath from a static member of CPluginSettings to a standalone
function.

Removed some directly-related occurrences of CString from CPluginDebug::Debug
and CPluginDebug::DebugClear.

[Oleksandr, 2014-08-05 12:06:08]

http://codereview.adblockplus.org/6288156869525504/diff/5741031244955648/src/...
File src/plugin/PluginSettings.h (right):

http://codereview.adblockplus.org/6288156869525504/diff/5741031244955648/src/...
src/plugin/PluginSettings.h:116: std::wstring GetDataPath(const std::wstring&
filename=L"");
If it's not a member of a class I'd move it elsewhere (Utils maybe?). But I
really don't mind having it as a static method. Even though it's not using any
static variables.

[sergei, 2014-08-05 12:12:11]

LGTM

[Eric, 2014-08-05 15:42:06]

Oleksandr, if you've got no objections to deferring moving code between files,
I'll push this.

http://codereview.adblockplus.org/6288156869525504/diff/5741031244955648/src/...
File src/plugin/PluginSettings.h (right):

http://codereview.adblockplus.org/6288156869525504/diff/5741031244955648/src/...
src/plugin/PluginSettings.h:116: std::wstring GetDataPath(const std::wstring&
filename=L"");
On 2014/08/05 12:06:08, Oleksandr wrote:
> If it's not a member of a class I'd move it elsewhere (Utils maybe?).

There are a bunch of static functions like this that don't need to be static.
They're not really utilities but rather more application-specific. They might
better go elsewhere, I agree, but it can also go in a separate change set,
similar to GetBrowserLanguage(), orphaned with the removal of CPluginSystem.

> But I
> really don't mind having it as a static method. Even though it's not using any
> static variables. 

It used to be in C++ that you'd use static class methods as a substitute for a
namespace. These days we have explicit namespaces, which is what I'd recommend
here.

The function in question is about the installation context of the plugin. Since
installation tends to act like global variables, with all their attendant
issues, it's usually a good idea to mark its occurrences somehow.

We can make all these decisions later, though, both what file to put the code it
and what namespace to use.

[Oleksandr, 2014-08-05 16:53:46]

ok. LGTM
On 2014/08/05 15:42:06, Eric wrote:
> Oleksandr, if you've got no objections to deferring moving code between files,
> I'll push this.
> 
>
http://codereview.adblockplus.org/6288156869525504/diff/5741031244955648/src/...
> File src/plugin/PluginSettings.h (right):
> 
>
http://codereview.adblockplus.org/6288156869525504/diff/5741031244955648/src/...
> src/plugin/PluginSettings.h:116: std::wstring GetDataPath(const std::wstring&
> filename=L"");
> On 2014/08/05 12:06:08, Oleksandr wrote:
> > If it's not a member of a class I'd move it elsewhere (Utils maybe?).
> 
> There are a bunch of static functions like this that don't need to be static.
> They're not really utilities but rather more application-specific. They might
> better go elsewhere, I agree, but it can also go in a separate change set,
> similar to GetBrowserLanguage(), orphaned with the removal of CPluginSystem.
> 
> > But I
> > really don't mind having it as a static method. Even though it's not using
any
> > static variables. 
> 
> It used to be in C++ that you'd use static class methods as a substitute for a
> namespace. These days we have explicit namespaces, which is what I'd recommend
> here.
> 
> The function in question is about the installation context of the plugin.
Since
> installation tends to act like global variables, with all their attendant
> issues, it's usually a good idea to mark its occurrences somehow.
> 
> We can make all these decisions later, though, both what file to put the code
it
> and what namespace to use.