Add an AddExtension() method in FilePath

base/file_path.cc

Index: base/file_path.cc
diff --git a/base/file_path.cc b/base/file_path.cc
index 3b1fca4b4ed6c7e8c0e52bf643211d66204a3c97..dfa3525db7462f663b7083646f1872bb3a214a77 100644
--- a/base/file_path.cc
+++ b/base/file_path.cc
@@ -159,6 +159,21 @@ StringType::size_type ExtensionSeparatorPosition(const StringType& path) {
   return last_dot;
 }
 
+// Returns true if path is "", ".", or "..".
+bool IsEmptyOrSpecialCase(const StringType& path) {
+  if (path.empty())
+    return true;
+  if (*(path.end() - 1) == FilePath::kExtensionSeparator) {

[Mark Mentovai, 2012/04/19 20:59:09]

What’s this actually buy, other than making the code obtuse? Why don’t you just
get rid of this check and proceed directly to the
kCurrentDirectory/kParentDirectory check?

[Devlin, 2012/04/21 01:37:36]

I didn't write this portion of the code (just refactored it), and noticed the
same thing.  My only guess was that it was to save that minor amount of work
since comparing the end character to see if a special case is even a concern is
faster than comparing the entirety of the path.  I'm not sure this will ever be
a concern (I'm unfamiliar with any instances of where a FilePath would be long
enough for this to matter), but I've realized a general pattern of people
preferring to not alter what is not broken.  If you like, I can simplify it.

+    // Special case "." and ".."
+    if (path == FilePath::kCurrentDirectory ||
+        path == FilePath::kParentDirectory) {
+      return true;
+    }
+  }
+
+  return false;
+}
+
 }  // namespace
 
 FilePath::FilePath() {
@@ -378,15 +393,8 @@ FilePath FilePath::InsertBeforeExtension(const StringType& suffix) const {
   if (path_.empty())
     return FilePath();
 
-  StringType base = BaseName().value();
-  if (base.empty())
+  if (IsEmptyOrSpecialCase(BaseName().value()))
     return FilePath();
-  if (*(base.end() - 1) == kExtensionSeparator) {
-    // Special case "." and ".."
-    if (base == kCurrentDirectory || base == kParentDirectory) {
-      return FilePath();
-    }
-  }
 
   StringType ext = Extension();
   StringType ret = RemoveExtension().value();
@@ -405,19 +413,32 @@ FilePath FilePath::InsertBeforeExtensionASCII(const base::StringPiece& suffix)
 #endif
 }
 
-FilePath FilePath::ReplaceExtension(const StringType& extension) const {
+FilePath FilePath::AddExtension(const StringType& extension) const {
   if (path_.empty())
     return FilePath();
 
-  StringType base = BaseName().value();
-  if (base.empty())
+  if (IsEmptyOrSpecialCase(BaseName().value()))
     return FilePath();
-  if (*(base.end() - 1) == kExtensionSeparator) {
-    // Special case "." and ".."
-    if (base == kCurrentDirectory || base == kParentDirectory) {
-      return FilePath();
-    }
+
+  // If the new extension is "" or ".", then just return the current FilePath.
+  if (extension.empty() || extension == StringType(1, kExtensionSeparator))
+    return *this;
+
+  StringType str = path_;
+  if (extension[0] != kExtensionSeparator &&
+      *(str.end() - 1) != kExtensionSeparator) {
+    str.append(1, kExtensionSeparator);
   }
+  str.append(extension);
+  return FilePath(str);
+}
+
+FilePath FilePath::ReplaceExtension(const StringType& extension) const {
+  if (path_.empty())
+    return FilePath();
+
+  if (IsEmptyOrSpecialCase(BaseName().value()))
+    return FilePath();
 
   FilePath no_ext = RemoveExtension();
   // If the new extension is "" or ".", then just remove the current extension.