Begin the first small step towards using FilePath everywhere:

Begin the first small step towards using FilePath everywhere:
- Add some transition APIs.
- Start migrating some code to transition APIs.

[Evan Martin, 2008-10-28 05:36:04]

Mark, I apologize for sullying your nice FilePath API, but getting everyone to
use it will take a lot of time.

Please also note:
 - The Mac changes are a wild guess and haven't been compiled.  Suggestions
welcome; I'll make sure the try bot likes 'em before committing.
 - I'll also need to fix the Windows half of file_util.

[Mark Mentovai, 2008-10-28 13:40:15]

This looks pretty good to me.  I'll take a look at the rest of the diffs when
you've got the Windows bits complete and everything tested.  Thanks for tackling
this.

http://codereview.chromium.org/8825/diff/33/35
File base/base_paths_linux.cc (right):

http://codereview.chromium.org/8825/diff/33/35#newcode38
Line 38: path = path.Append("../..");
Can you append FilePath::kParentDirectory twice?

http://codereview.chromium.org/8825/diff/33/36
File base/file_path.cc (right):

http://codereview.chromium.org/8825/diff/33/36#newcode7
Line 7: #include "base/string_piece.h"
Since these are for the hack and we can take them out when the hack is gone, you
can put a comment up here or down below by the hack code to remind us to remove
them in the future.

http://codereview.chromium.org/8825/diff/33/37
File base/file_path.h (right):

http://codereview.chromium.org/8825/diff/33/37#newcode152
Line 152: // be removed once all code is fixed.
The last sentence should be reworded along the lines of "TODO(port): eliminate
callers and remove"

http://codereview.chromium.org/8825/diff/33/41
File base/file_util_mac.mm (right):

http://codereview.chromium.org/8825/diff/33/41#newcode22
Line 22: *path = FilePath([tmp cStringUsingEncoding:NSUTF8StringEncoding]);
There's a convenience method to get a UTF-8 string from an NSString*.  You can
use:

*path = FilePath([tmp UTF8String]);

To my eyes, that's slightly more readable.

http://codereview.chromium.org/8825/diff/33/42
File base/file_util_posix.cc (right):

http://codereview.chromium.org/8825/diff/33/42#newcode295
Line 295: for (FilePath path = full_path; path.value() != "/"; path =
path.DirName()) {
This will never terminate if you pass it a relative full_path.

http://codereview.chromium.org/8825/diff/33/43
File base/file_util_unittest.cc (right):

http://codereview.chromium.org/8825/diff/33/43#newcode710
Line 710: FilePath test_path =
You can make this just a be a sequence of Append calls to get rid of the
#ifdefs, but I guess we might want to test appending strings with separators in
them.

http://codereview.chromium.org/8825/diff/33/43#newcode855
Line 855: FilePath file2_rel = dir2.Append(FILE_PATH_LITERAL(".."))
Instead of .. can you use the constant from FilePath?

[Avi (use Gerrit), 2008-10-28 14:53:16]

http://codereview.chromium.org/8825/diff/33/41
File base/file_util_mac.mm (right):

http://codereview.chromium.org/8825/diff/33/41#newcode22
Line 22: *path = FilePath([tmp cStringUsingEncoding:NSUTF8StringEncoding]);
On 2008/10/28 13:40:15, Mark Mentovai wrote:
> *path = FilePath([tmp UTF8String]);
[tmp fileSystemRepresentation]?

[Evan Martin, 2008-10-28 15:42:52]

Ok, new snapshot up with review comments addressed.
I will ping you again when I have it tested on Windows.

[Evan Martin, 2008-10-28 22:38:26]

Windows fixes now uploaded.  It passes the base unit tests; still compiling the
rest, but I think this is ready for review.

[Mark Mentovai, 2008-10-28 23:02:33]

LGTM conditioned on testing on a Mac

http://codereview.chromium.org/8825/diff/224/110
File base/file_util_mac.mm (right):

http://codereview.chromium.org/8825/diff/224/110#newcode22
Line 22: *path = FilePath([tmp fileSystemPathRepresentation]);
fileSystemRepresentation

http://codereview.chromium.org/8825/diff/224/111
File base/file_util_posix.cc (right):

http://codereview.chromium.org/8825/diff/224/111#newcode297
Line 297: path.value() != last_path.value(); path = path.DirName()) {
You never reassign to last_path, so now this never terminates even for an
absolute path.

http://codereview.chromium.org/8825/diff/224/114
File base/icu_util.cc (right):

http://codereview.chromium.org/8825/diff/224/114#newcode84
Line 84: base::SysWideToNativeMB(data_path.ToWStringHack()).c_str());
Ew.

OK, I guess this isn't too much worse than what was there before, but since this
is Linux-only, can't you just use data_path.value().c_str() directly?

[Evan Martin, 2008-10-30 22:57:31]

> http://codereview.chromium.org/8825/diff/224/114#newcode84
> Line 84: base::SysWideToNativeMB(data_path.ToWStringHack()).c_str());
> Ew.
> 
> OK, I guess this isn't too much worse than what was there before, but since
this
> is Linux-only, can't you just use data_path.value().c_str() directly?

At least the #ifdef isn't Linux-specific.  Since I'm using the Hack() function,
it'll be easy to grep out in the future.

New snapshot uploaded.  I think I'm gonna get Tony to look at it now unless
you're eager to continue looking.

[tony, 2008-10-30 23:33:39]

Wow, LGTM.  Thanks for starting this process.

On 2008/10/30 22:57:31, Evan Martin wrote:
> > http://codereview.chromium.org/8825/diff/224/114#newcode84
> > Line 84: base::SysWideToNativeMB(data_path.ToWStringHack()).c_str());
> > Ew.
> > 
> > OK, I guess this isn't too much worse than what was there before, but since
> this
> > is Linux-only, can't you just use data_path.value().c_str() directly?
> 
> At least the #ifdef isn't Linux-specific.  Since I'm using the Hack()
function,
> it'll be easy to grep out in the future.
> 
> New snapshot uploaded.  I think I'm gonna get Tony to look at it now unless
> you're eager to continue looking.