Now BOM is preserved for UTF-8 files (#13477)

* Now BOM is preserved for UTF-8 files * Added error strings for failure in encode/decode and utf-16 * Removed utf-16 from encodings list * Addressed review comments
adobe · Jun 27, 2017 · 8ecb5ed · 8ecb5ed
1 parent 3aa8ef6
commit 8ecb5ed
Show file tree

Hide file tree

Showing 7 changed files with 39 additions and 11 deletions.
diff --git a/src/file/FileUtils.js b/src/file/FileUtils.js
@@ -188,6 +188,12 @@ define(function (require, exports, module) {
             result = Strings.UNSUPPORTED_ENCODING_ERR;
         } else if (name === FileSystemError.EXCEEDS_MAX_FILE_SIZE) {
             result = StringUtils.format(Strings.EXCEEDS_MAX_FILE_SIZE, MAX_FILE_SIZE_MB);
+        } else if (name === FileSystemError.ENCODE_FILE_FAILED) {
+            result = Strings.ENCODE_FILE_FAILED_ERR;
+        } else if (name === FileSystemError.DECODE_FILE_FAILED) {
+            result = Strings.DECODE_FILE_FAILED_ERR;
+        } else if (name === FileSystemError.UNSUPPORTED_UTF16_ENCODING) {
+            result = Strings.UNSUPPORTED_UTF16_ENCODING_ERR;
         } else {
             result = StringUtils.format(Strings.GENERIC_ERROR, name);
         }

diff --git a/src/filesystem/File.js b/src/filesystem/File.js
@@ -63,6 +63,13 @@ define(function (require, exports, module) {
      */
     File.prototype._encoding = null;
 
+    /**
+     * BOM detected by brackets-shell
+     * @private
+     * @type {?bool}
+     */
+    File.prototype._preserveBOM = false;
+
     /**
      * Consistency hash for this file. Reads and writes update this value, and
      * writes confirm the hash before overwriting existing files. The type of
@@ -112,7 +119,7 @@ define(function (require, exports, module) {
             options.stat = this._stat;
         }
 
-        this._impl.readFile(this._path, options, function (err, data, encoding, stat) {
+        this._impl.readFile(this._path, options, function (err, data, encoding, preserveBOM, stat) {
             if (err) {
                 this._clearCachedData();
                 callback(err);
@@ -122,6 +129,7 @@ define(function (require, exports, module) {
             // Always store the hash
             this._hash = stat._hash;
             this._encoding = encoding;
+            this._preserveBOM = preserveBOM;
 
             // Only cache data for watched files
             if (watched) {
@@ -158,7 +166,10 @@ define(function (require, exports, module) {
             options.expectedHash = this._hash;
             options.expectedContents = this._contents;
         }
-        options.encoding = this._encoding || "utf8";
+        if (!options.encoding) {
+            options.encoding = this._encoding || "utf8";
+        }
+        options.preserveBOM = this._preserveBOM;
 
         // Block external change events until after the write has finished
         this._fileSystem._beginChange();

diff --git a/src/filesystem/FileSystemError.js b/src/filesystem/FileSystemError.js
@@ -48,7 +48,10 @@ define(function (require, exports, module) {
         CONTENTS_MODIFIED           : "ContentsModified",
         ROOT_NOT_WATCHED            : "RootNotBeingWatched",
         EXCEEDS_MAX_FILE_SIZE       : "ExceedsMaxFileSize",
-        NETWORK_DRIVE_NOT_SUPPORTED : "NetworkDriveNotSupported"
+        NETWORK_DRIVE_NOT_SUPPORTED : "NetworkDriveNotSupported",
+        ENCODE_FILE_FAILED          : "EncodeFileFailed",
+        DECODE_FILE_FAILED          : "DecodeFileFailed",
+        UNSUPPORTED_UTF16_ENCODING  : "UnsupportedUTF16Encoding"
 
         // FUTURE: Add remote connection errors: timeout, not logged in, connection err, etc.
     };

diff --git a/src/filesystem/impls/appshell/AppshellFileSystem.js b/src/filesystem/impls/appshell/AppshellFileSystem.js
@@ -160,6 +160,12 @@ define(function (require, exports, module) {
             return FileSystemError.OUT_OF_SPACE;
         case appshell.fs.ERR_FILE_EXISTS:
             return FileSystemError.ALREADY_EXISTS;
+        case appshell.fs.ERR_ENCODE_FILE_FAILED:
+            return FileSystemError.ENCODE_FILE_FAILED;
+        case appshell.fs.ERR_ENCODE_FILE_FAILED:
+            return FileSystemError.DECODE_FILE_FAILED;
+        case appshell.fs.ERR_UNSUPPORTED_UTF16_ENCODING:
+            return FileSystemError.UNSUPPORTED_UTF16_ENCODING;
         }
         return FileSystemError.UNKNOWN;
     }
@@ -363,11 +369,11 @@ define(function (require, exports, module) {
             if (stat.size > (FileUtils.MAX_FILE_SIZE)) {
                 callback(FileSystemError.EXCEEDS_MAX_FILE_SIZE);
             } else {
-                appshell.fs.readFile(path, encoding, function (_err, _data, encoding) {
+                appshell.fs.readFile(path, encoding, function (_err, _data, encoding, preserveBOM) {
                     if (_err) {
                         callback(_mapError(_err));
                     } else {
-                        callback(null, _data, encoding, stat);
+                        callback(null, _data, encoding, preserveBOM, stat);
                     }
                 });
             }
@@ -403,10 +409,11 @@ define(function (require, exports, module) {
      * @param {function(?string, FileSystemStats=, boolean)} callback
      */
     function writeFile(path, data, options, callback) {
-        var encoding = options.encoding || "utf8";
+        var encoding = options.encoding || "utf8",
+            preserveBOM = options.preserveBOM;
 
         function _finishWrite(created) {
-            appshell.fs.writeFile(path, data, encoding, function (err) {
+            appshell.fs.writeFile(path, data, encoding, preserveBOM, function (err) {
                 if (err) {
                     callback(_mapError(err));
                 } else {

diff --git a/src/nls/root/strings.js b/src/nls/root/strings.js
@@ -35,7 +35,10 @@ define({
     "NO_MODIFICATION_ALLOWED_ERR"       : "The target directory cannot be modified.",
     "NO_MODIFICATION_ALLOWED_ERR_FILE"  : "The permissions do not allow you to make modifications.",
     "CONTENTS_MODIFIED_ERR"             : "The file has been modified outside of {APP_NAME}.",
-    "UNSUPPORTED_ENCODING_ERR"          : "{APP_NAME} currently only supports UTF-8 encoded text files.",
+    "UNSUPPORTED_ENCODING_ERR"          : "Unknown encoding format",
+    "ENCODE_FILE_FAILED_ERR"            : "{APP_NAME} was not able to encode the contents of file.",
+    "DECODE_FILE_FAILED_ERR"            : "{APP_NAME} was not able to decode the contents of file.",
+    "UNSUPPORTED_UTF16_ENCODING_ERR"    : "{APP_NAME} currently doesn't support UTF-16 encoded text files.",
     "FILE_EXISTS_ERR"                   : "The file or directory already exists.",
     "FILE"                              : "file",
     "FILE_TITLE"                        : "File",

diff --git a/src/supported-encodings.json b/src/supported-encodings.json
@@ -31,8 +31,6 @@
     "KOI8-R",
     "KOI8-RU",
     "SHIFT_JIS",
-    "UTF-16BE",
-    "UTF-16LE",
     "UTF-8",
     "WINDOWS-1250",
     "WINDOWS-1251",

diff --git a/test/spec/MockFileSystemImpl.js b/test/spec/MockFileSystemImpl.js
@@ -141,7 +141,7 @@ define(function (require, exports, module) {
         if (!_model.exists(path)) {
             cb(FileSystemError.NOT_FOUND);
         } else {
-            cb(null, _model.readFile(path), "UTF-8", _model.stat(path));
+            cb(null, _model.readFile(path), "UTF-8", false, _model.stat(path));
         }
     }