Make FilePath::Append accept FilePath parameter. Patch by Paweł Hajdan jr
<phajdan.jr@gmail.com>
http://codereview.chromium.org/12907
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@6511 0039d316-1c4b-4281-b951-d872f2087c98
CrOS-Libchrome-Original-Commit: 1906c0367cceb072804f7fb355ab33595fa188bd
diff --git a/base/directory_watcher_unittest.cc b/base/directory_watcher_unittest.cc
index 5121287..39f64bb 100644
--- a/base/directory_watcher_unittest.cc
+++ b/base/directory_watcher_unittest.cc
@@ -6,12 +6,17 @@
#include <fstream>
+#include "build/build_config.h"
+
#include "base/file_path.h"
#include "base/file_util.h"
#include "base/logging.h"
#include "base/message_loop.h"
#include "base/path_service.h"
#include "base/string_util.h"
+#if defined(OS_WIN)
+#include "base/win_util.h"
+#endif
#include "testing/gtest/include/gtest/gtest.h"
// For tests where we wait a bit to verify nothing happened
@@ -125,8 +130,15 @@
// Verify that modifications to a subdirectory isn't noticed.
TEST_F(DirectoryWatcherTest, SubDir) {
- FilePath subdir = test_dir_.Append(FILE_PATH_LITERAL("SubDir"));
- ASSERT_TRUE(file_util::CreateDirectory(subdir.value()));
+#if defined(OS_WIN)
+ // Temporarily disabling test on Vista, see
+ // http://code.google.com/p/chromium/issues/detail?id=5072
+ // TODO: Enable this test, quickly.
+ if (win_util::GetWinVersion() == win_util::WINVERSION_VISTA)
+ return;
+#endif
+ FilePath subdir(FILE_PATH_LITERAL("SubDir"));
+ ASSERT_TRUE(file_util::CreateDirectory(test_dir_.Append(subdir)));
DirectoryWatcher watcher;
ASSERT_TRUE(watcher.Watch(test_dir_, this));
@@ -141,7 +153,7 @@
loop_.Run();
// We shouldn't have been notified and shouldn't have crashed.
- ASSERT_EQ(directory_mods_, 0);
+ ASSERT_EQ(0, directory_mods_);
}
namespace {
diff --git a/base/file_path.cc b/base/file_path.cc
index 98a4703..0856029 100644
--- a/base/file_path.cc
+++ b/base/file_path.cc
@@ -3,6 +3,7 @@
// found in the LICENSE file.
#include "base/file_path.h"
+#include "base/logging.h"
// These includes are just for the *Hack functions, and should be removed
// when those functions are removed.
@@ -18,6 +19,48 @@
const FilePath::CharType FilePath::kCurrentDirectory[] = FILE_PATH_LITERAL(".");
const FilePath::CharType FilePath::kParentDirectory[] = FILE_PATH_LITERAL("..");
+namespace {
+
+// If this FilePath contains a drive letter specification, returns the
+// position of the last character of the drive letter specification,
+// otherwise returns npos. This can only be true on Windows, when a pathname
+// begins with a letter followed by a colon. On other platforms, this always
+// returns npos.
+FilePath::StringType::size_type FindDriveLetter(
+ const FilePath::StringType& path) {
+#if defined(FILE_PATH_USES_DRIVE_LETTERS)
+ // This is dependent on an ASCII-based character set, but that's a
+ // reasonable assumption. iswalpha can be too inclusive here.
+ if (path.length() >= 2 && path[1] == L':' &&
+ ((path[0] >= L'A' && path[0] <= L'Z') ||
+ (path[0] >= L'a' && path[0] <= L'z'))) {
+ return 1;
+ }
+ return FilePath::StringType::npos;
+#else // FILE_PATH_USES_DRIVE_LETTERS
+ return FilePath::StringType::npos;
+#endif // FILE_PATH_USES_DRIVE_LETTERS
+}
+
+bool IsPathAbsolute(const FilePath::StringType& path) {
+#if defined(FILE_PATH_USES_DRIVE_LETTERS)
+ FilePath::StringType::size_type letter = FindDriveLetter(path);
+ if (letter != FilePath::StringType::npos) {
+ // Look for a separator right after the drive specification.
+ return path.length() > letter + 1 &&
+ FilePath::IsSeparator(path[letter + 1]);
+ }
+ // Look for a pair of leading separators.
+ return path.length() > 1 &&
+ FilePath::IsSeparator(path[0]) && FilePath::IsSeparator(path[1]);
+#else // FILE_PATH_USES_DRIVE_LETTERS
+ // Look for a separator in the first position.
+ return path.length() > 0 && FilePath::IsSeparator(path[0]);
+#endif // FILE_PATH_USES_DRIVE_LETTERS
+}
+
+} // namespace
+
bool FilePath::IsSeparator(CharType character) {
for (size_t i = 0; i < arraysize(kSeparators) - 1; ++i) {
if (character == kSeparators[i]) {
@@ -40,7 +83,7 @@
// is no drive letter, as will always be the case on platforms which do not
// support drive letters, letter will be npos, or -1, so the comparisons and
// resizes below using letter will still be valid.
- StringType::size_type letter = new_path.FindDriveLetter();
+ StringType::size_type letter = FindDriveLetter(new_path.path_);
StringType::size_type last_separator =
new_path.path_.find_last_of(kSeparators, StringType::npos,
@@ -73,7 +116,7 @@
new_path.StripTrailingSeparators();
// The drive letter, if any, is always stripped.
- StringType::size_type letter = new_path.FindDriveLetter();
+ StringType::size_type letter = FindDriveLetter(new_path.path_);
if (letter != StringType::npos) {
new_path.path_.erase(0, letter + 1);
}
@@ -92,6 +135,7 @@
}
FilePath FilePath::Append(const FilePath::StringType& component) const {
+ DCHECK(!IsPathAbsolute(component));
if (path_.compare(kCurrentDirectory) == 0) {
// Append normally doesn't do any normalization, but as a special case,
// when appending to kCurrentDirectory, just return a new path for the
@@ -116,7 +160,7 @@
if (!IsSeparator(new_path.path_[new_path.path_.length() - 1])) {
// Don't append a separator if the path is just a drive letter.
- if (new_path.FindDriveLetter() + 1 != new_path.path_.length()) {
+ if (FindDriveLetter(new_path.path_) + 1 != new_path.path_.length()) {
new_path.path_.append(1, kSeparators[0]);
}
}
@@ -126,34 +170,12 @@
return new_path;
}
-FilePath::StringType::size_type FilePath::FindDriveLetter() const {
-#if defined(FILE_PATH_USES_DRIVE_LETTERS)
- // This is dependent on an ASCII-based character set, but that's a
- // reasonable assumption. iswalpha can be too inclusive here.
- if (path_.length() >= 2 && path_[1] == L':' &&
- ((path_[0] >= L'A' && path_[0] <= L'Z') ||
- (path_[0] >= L'a' && path_[0] <= L'z'))) {
- return 1;
- }
- return StringType::npos;
-#else // FILE_PATH_USES_DRIVE_LETTERS
- return StringType::npos;
-#endif // FILE_PATH_USES_DRIVE_LETTERS
+FilePath FilePath::Append(const FilePath& component) const {
+ return Append(component.value());
}
bool FilePath::IsAbsolute() const {
-#if defined(FILE_PATH_USES_DRIVE_LETTERS)
- StringType::size_type letter = FindDriveLetter();
- if (letter != StringType::npos) {
- // Look for a separator right after the drive specification.
- return path_.length() > letter + 1 && IsSeparator(path_[letter + 1]);
- }
- // Look for a pair of leading separators.
- return path_.length() > 1 && IsSeparator(path_[0]) && IsSeparator(path_[1]);
-#else // FILE_PATH_USES_DRIVE_LETTERS
- // Look for a separator in the first position.
- return path_.length() > 0 && IsSeparator(path_[0]);
-#endif // FILE_PATH_USES_DRIVE_LETTERS
+ return IsPathAbsolute(path_);
}
#if defined(OS_POSIX)
@@ -185,7 +207,7 @@
// letter, start will be set appropriately to prevent stripping the first
// separator following the drive letter, if a separator immediately follows
// the drive letter.
- StringType::size_type start = FindDriveLetter() + 2;
+ StringType::size_type start = FindDriveLetter(path_) + 2;
StringType::size_type last_stripped = StringType::npos;
for (StringType::size_type pos = path_.length();
diff --git a/base/file_path.h b/base/file_path.h
index fc55a7a..fb2cc57 100644
--- a/base/file_path.h
+++ b/base/file_path.h
@@ -146,6 +146,7 @@
// only to |component| is returned. |component| must be a relative path;
// it is an error to pass an absolute path.
FilePath Append(const StringType& component) const WARN_UNUSED_RESULT;
+ FilePath Append(const FilePath& component) const WARN_UNUSED_RESULT;
// Returns true if this FilePath contains an absolute path. On Windows, an
// absolute path begins with either a drive letter specification followed by
@@ -168,13 +169,6 @@
std::wstring ToWStringHack() const;
private:
- // If this FilePath contains a drive letter specification, returns the
- // position of the last character of the drive letter specification,
- // otherwise returns npos. This can only be true on Windows, when a pathname
- // begins with a letter followed by a colon. On other platforms, this always
- // returns npos.
- StringType::size_type FindDriveLetter() const;
-
// Remove trailing separators from this object. If the path is absolute, it
// will never be stripped any more than to refer to the absolute root
// directory, so "////" will become "/", not "". A leading pair of
diff --git a/base/file_path_unittest.cc b/base/file_path_unittest.cc
index 0dee543..ef6239f 100644
--- a/base/file_path_unittest.cc
+++ b/base/file_path_unittest.cc
@@ -273,8 +273,11 @@
for (size_t i = 0; i < arraysize(cases); ++i) {
FilePath root(cases[i].inputs[0]);
FilePath::StringType leaf(cases[i].inputs[1]);
- FilePath observed = root.Append(leaf);
- EXPECT_EQ(FilePath::StringType(cases[i].expected), observed.value()) <<
+ FilePath observed_str = root.Append(leaf);
+ EXPECT_EQ(FilePath::StringType(cases[i].expected), observed_str.value()) <<
+ "i: " << i << ", root: " << root.value() << ", leaf: " << leaf;
+ FilePath observed_path = root.Append(FilePath(leaf));
+ EXPECT_EQ(FilePath::StringType(cases[i].expected), observed_path.value()) <<
"i: " << i << ", root: " << root.value() << ", leaf: " << leaf;
}
}