You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@cordova.apache.org by GitBox <gi...@apache.org> on 2020/07/02 12:05:22 UTC

[GitHub] [cordova-plugin-file] timbru31 opened a new pull request #405: refactor(eslint): use cordova-eslint /w fix

timbru31 opened a new pull request #405:
URL: https://github.com/apache/cordova-plugin-file/pull/405


   ### Motivation and Context
   
   Consistency across our codebase.
   
   This is a bigger one. Changes of interest are noted as a review comment by myself.
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org


[GitHub] [cordova-plugin-file] timbru31 commented on a change in pull request #405: refactor(eslint): use cordova-eslint /w fix

Posted by GitBox <gi...@apache.org>.
timbru31 commented on a change in pull request #405:
URL: https://github.com/apache/cordova-plugin-file/pull/405#discussion_r448953140



##########
File path: src/browser/FileProxy.js
##########
@@ -521,7 +581,7 @@
             }
 
             // support for encodeURI
-            if (/\%5/g.test(path) || /\%20/g.test(path)) {  // eslint-disable-line no-useless-escape
+            if (/%5/g.test(path) || /%20/g.test(path)) {

Review comment:
       Useless escape of `%`




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org


[GitHub] [cordova-plugin-file] timbru31 commented on a change in pull request #405: refactor(eslint): use cordova-eslint /w fix

Posted by GitBox <gi...@apache.org>.
timbru31 commented on a change in pull request #405:
URL: https://github.com/apache/cordova-plugin-file/pull/405#discussion_r448954852



##########
File path: src/browser/FileProxy.js
##########
@@ -56,11 +57,6 @@
     var File = require('./File');
 
     (function (exports, global) {
-        var indexedDB = global.indexedDB || global.mozIndexedDB;

Review comment:
       If desired, I'll re-add the IndexedDB check and adjust the error message.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org


[GitHub] [cordova-plugin-file] timbru31 commented on a change in pull request #405: refactor(eslint): use cordova-eslint /w fix

Posted by GitBox <gi...@apache.org>.
timbru31 commented on a change in pull request #405:
URL: https://github.com/apache/cordova-plugin-file/pull/405#discussion_r448954438



##########
File path: www/android/FileSystem.js
##########
@@ -17,9 +17,11 @@
  * specific language governing permissions and limitations
  * under the License.
  *
-*/
+ */
 
-FILESYSTEM_PROTOCOL = 'cdvfile'; // eslint-disable-line no-undef

Review comment:
       adding a `var` keyword should have the same effect as omitting it. Using ES5 notation to retain compatibility. Same change applies to other files.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org


[GitHub] [cordova-plugin-file] timbru31 commented on a change in pull request #405: refactor(eslint): use cordova-eslint /w fix

Posted by GitBox <gi...@apache.org>.
timbru31 commented on a change in pull request #405:
URL: https://github.com/apache/cordova-plugin-file/pull/405#discussion_r448952819



##########
File path: src/browser/FileProxy.js
##########
@@ -81,13 +77,10 @@
 
         var unicodeLastChar = 65535;
 
-    /** * Exported functionality ***/
+        /** * Exported functionality ***/
 
         exports.requestFileSystem = function (successCallback, errorCallback, args) {
             var type = args[0];
-            // Size is ignored since IDB filesystem size depends
-            // on browser implementation and can't be set up by user
-            var size = args[1]; // eslint-disable-line no-unused-vars

Review comment:
       No reason to allocate memory if the variable is not used.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org


[GitHub] [cordova-plugin-file] erisu commented on a change in pull request #405: refactor(eslint): use cordova-eslint /w fix

Posted by GitBox <gi...@apache.org>.
erisu commented on a change in pull request #405:
URL: https://github.com/apache/cordova-plugin-file/pull/405#discussion_r493143602



##########
File path: www/android/FileSystem.js
##########
@@ -17,9 +17,11 @@
  * specific language governing permissions and limitations
  * under the License.
  *
-*/
+ */
 
-FILESYSTEM_PROTOCOL = 'cdvfile'; // eslint-disable-line no-undef

Review comment:
       Yes, add var.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org


[GitHub] [cordova-plugin-file] timbru31 commented on a change in pull request #405: refactor(eslint): use cordova-eslint /w fix

Posted by GitBox <gi...@apache.org>.
timbru31 commented on a change in pull request #405:
URL: https://github.com/apache/cordova-plugin-file/pull/405#discussion_r493335396



##########
File path: src/browser/FileProxy.js
##########
@@ -81,13 +77,10 @@
 
         var unicodeLastChar = 65535;
 
-    /** * Exported functionality ***/
+        /** * Exported functionality ***/
 
         exports.requestFileSystem = function (successCallback, errorCallback, args) {
             var type = args[0];
-            // Size is ignored since IDB filesystem size depends
-            // on browser implementation and can't be set up by user
-            var size = args[1]; // eslint-disable-line no-unused-vars

Review comment:
       The variable? Already did. I just wanted to highlight this change, because the diff is pretty big and lots for "formatting".




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org


[GitHub] [cordova-plugin-file] erisu commented on a change in pull request #405: refactor(eslint): use cordova-eslint /w fix

Posted by GitBox <gi...@apache.org>.
erisu commented on a change in pull request #405:
URL: https://github.com/apache/cordova-plugin-file/pull/405#discussion_r493142989



##########
File path: src/browser/FileProxy.js
##########
@@ -81,13 +77,10 @@
 
         var unicodeLastChar = 65535;
 
-    /** * Exported functionality ***/
+        /** * Exported functionality ***/
 
         exports.requestFileSystem = function (successCallback, errorCallback, args) {
             var type = args[0];
-            // Size is ignored since IDB filesystem size depends
-            // on browser implementation and can't be set up by user
-            var size = args[1]; // eslint-disable-line no-unused-vars

Review comment:
       remove it.

##########
File path: src/browser/FileProxy.js
##########
@@ -141,103 +147,121 @@
             // Create an absolute path if we were handed a relative one.
             path = resolveToFullPath_(fullPath, path);
 
-            idb_.get(path.storagePath, function (fileEntry) {
-                if (options.create === true && options.exclusive === true && fileEntry) {
-                    // If create and exclusive are both true, and the path already exists,
-                    // getFile must fail.
+            idb_.get(
+                path.storagePath,
+                function (fileEntry) {
+                    if (options.create === true && options.exclusive === true && fileEntry) {
+                        // If create and exclusive are both true, and the path already exists,
+                        // getFile must fail.
 
-                    if (errorCallback) {
-                        errorCallback(FileError.PATH_EXISTS_ERR);
-                    }
-                } else if (options.create === true && !fileEntry) {
-                    // If create is true, the path doesn't exist, and no other error occurs,
-                    // getFile must create it as a zero-length file and return a corresponding
-                    // FileEntry.
-                    var newFileEntry = new FileEntry(path.fileName, path.fullPath, new FileSystem(path.fsName, fs_.root));
-
-                    newFileEntry.file_ = new MyFile({
-                        size: 0,
-                        name: newFileEntry.name,
-                        lastModifiedDate: new Date(),
-                        storagePath: path.storagePath
-                    });
-
-                    idb_.put(newFileEntry, path.storagePath, successCallback, errorCallback);
-                } else if (options.create === true && fileEntry) {
-                    if (fileEntry.isFile) {
-                        // Overwrite file, delete then create new.
-                        idb_['delete'](path.storagePath, function () {
-                            var newFileEntry = new FileEntry(path.fileName, path.fullPath, new FileSystem(path.fsName, fs_.root));
-
-                            newFileEntry.file_ = new MyFile({
-                                size: 0,
-                                name: newFileEntry.name,
-                                lastModifiedDate: new Date(),
-                                storagePath: path.storagePath
-                            });
-
-                            idb_.put(newFileEntry, path.storagePath, successCallback, errorCallback);
-                        }, errorCallback);
-                    } else {
                         if (errorCallback) {
-                            errorCallback(FileError.INVALID_MODIFICATION_ERR);
+                            errorCallback(FileError.PATH_EXISTS_ERR);
                         }
-                    }
-                } else if ((!options.create || options.create === false) && !fileEntry) {
-                    // If create is not true and the path doesn't exist, getFile must fail.
-                    if (errorCallback) {
-                        errorCallback(FileError.NOT_FOUND_ERR);
-                    }
-                } else if ((!options.create || options.create === false) && fileEntry &&
-                    fileEntry.isDirectory) {
-                    // If create is not true and the path exists, but is a directory, getFile
-                    // must fail.
-                    if (errorCallback) {
-                        errorCallback(FileError.TYPE_MISMATCH_ERR);
-                    }
-                } else {
-                    // Otherwise, if no other error occurs, getFile must return a FileEntry
-                    // corresponding to path.
+                    } else if (options.create === true && !fileEntry) {
+                        // If create is true, the path doesn't exist, and no other error occurs,
+                        // getFile must create it as a zero-length file and return a corresponding
+                        // FileEntry.
+                        var newFileEntry = new FileEntry(path.fileName, path.fullPath, new FileSystem(path.fsName, fs_.root));
+
+                        newFileEntry.file_ = new MyFile({
+                            size: 0,
+                            name: newFileEntry.name,
+                            lastModifiedDate: new Date(),
+                            storagePath: path.storagePath
+                        });
+
+                        idb_.put(newFileEntry, path.storagePath, successCallback, errorCallback);
+                    } else if (options.create === true && fileEntry) {
+                        if (fileEntry.isFile) {
+                            // Overwrite file, delete then create new.
+                            idb_.delete(
+                                path.storagePath,
+                                function () {
+                                    var newFileEntry = new FileEntry(path.fileName, path.fullPath, new FileSystem(path.fsName, fs_.root));
+
+                                    newFileEntry.file_ = new MyFile({
+                                        size: 0,
+                                        name: newFileEntry.name,
+                                        lastModifiedDate: new Date(),
+                                        storagePath: path.storagePath
+                                    });
+
+                                    idb_.put(newFileEntry, path.storagePath, successCallback, errorCallback);
+                                },
+                                errorCallback
+                            );
+                        } else {
+                            if (errorCallback) {
+                                errorCallback(FileError.INVALID_MODIFICATION_ERR);
+                            }
+                        }
+                    } else if ((!options.create || options.create === false) && !fileEntry) {
+                        // If create is not true and the path doesn't exist, getFile must fail.
+                        if (errorCallback) {
+                            errorCallback(FileError.NOT_FOUND_ERR);
+                        }
+                    } else if ((!options.create || options.create === false) && fileEntry && fileEntry.isDirectory) {
+                        // If create is not true and the path exists, but is a directory, getFile
+                        // must fail.
+                        if (errorCallback) {
+                            errorCallback(FileError.TYPE_MISMATCH_ERR);
+                        }
+                    } else {
+                        // Otherwise, if no other error occurs, getFile must return a FileEntry
+                        // corresponding to path.
 
-                    successCallback(fileEntryFromIdbEntry(fileEntry));
-                }
-            }, errorCallback);
+                        successCallback(fileEntryFromIdbEntry(fileEntry));
+                    }
+                },
+                errorCallback
+            );
         };
 
         exports.getFileMetadata = function (successCallback, errorCallback, args) {
             var fullPath = args[0];
 
-            exports.getFile(function (fileEntry) {
-                successCallback(new File(fileEntry.file_.name, fileEntry.fullPath, '', fileEntry.file_.lastModifiedDate,
-                    fileEntry.file_.size));
-            }, errorCallback, [fullPath, null]);
+            exports.getFile(
+                function (fileEntry) {
+                    successCallback(
+                        new File(fileEntry.file_.name, fileEntry.fullPath, '', fileEntry.file_.lastModifiedDate, fileEntry.file_.size)
+                    );
+                },
+                errorCallback,
+                [fullPath, null]
+            );
         };
 
         exports.getMetadata = function (successCallback, errorCallback, args) {
-            exports.getFile(function (fileEntry) {
-                successCallback(
-                    {
+            exports.getFile(
+                function (fileEntry) {
+                    successCallback({
                         modificationTime: fileEntry.file_.lastModifiedDate,
                         size: fileEntry.file_.lastModifiedDate
                     });
-            }, errorCallback, args);
+                },
+                errorCallback,
+                args
+            );
         };
 
         exports.setMetadata = function (successCallback, errorCallback, args) {
             var fullPath = args[0];
             var metadataObject = args[1];
 
-            exports.getFile(function (fileEntry) {
-                fileEntry.file_.lastModifiedDate = metadataObject.modificationTime;
-                idb_.put(fileEntry, fileEntry.file_.storagePath, successCallback, errorCallback);
-            }, errorCallback, [fullPath, null]);
+            exports.getFile(
+                function (fileEntry) {
+                    fileEntry.file_.lastModifiedDate = metadataObject.modificationTime;
+                    idb_.put(fileEntry, fileEntry.file_.storagePath, successCallback, errorCallback);
+                },
+                errorCallback,
+                [fullPath, null]
+            );
         };
 
         exports.write = function (successCallback, errorCallback, args) {
             var fileName = args[0];
             var data = args[1];
             var position = args[2];
-            var isBinary = args[3]; // eslint-disable-line no-unused-vars

Review comment:
       remove it.

##########
File path: src/browser/FileProxy.js
##########
@@ -521,7 +581,7 @@
             }
 
             // support for encodeURI
-            if (/\%5/g.test(path) || /\%20/g.test(path)) {  // eslint-disable-line no-useless-escape
+            if (/%5/g.test(path) || /%20/g.test(path)) {

Review comment:
       remove it.

##########
File path: src/browser/FileProxy.js
##########
@@ -56,11 +57,6 @@
     var File = require('./File');
 
     (function (exports, global) {
-        var indexedDB = global.indexedDB || global.mozIndexedDB;

Review comment:
       Do not remove it in this PR.

##########
File path: www/android/FileSystem.js
##########
@@ -17,9 +17,11 @@
  * specific language governing permissions and limitations
  * under the License.
  *
-*/
+ */
 
-FILESYSTEM_PROTOCOL = 'cdvfile'; // eslint-disable-line no-undef

Review comment:
       Yes, add var.

##########
File path: www/android/FileSystem.js
##########
@@ -17,9 +17,11 @@
  * specific language governing permissions and limitations
  * under the License.
  *
-*/
+ */
 
-FILESYSTEM_PROTOCOL = 'cdvfile'; // eslint-disable-line no-undef

Review comment:
       Yes, add `var` and remove the `eslint-disable-line no-undef`




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org


[GitHub] [cordova-plugin-file] erisu commented on a change in pull request #405: refactor(eslint): use cordova-eslint /w fix

Posted by GitBox <gi...@apache.org>.
erisu commented on a change in pull request #405:
URL: https://github.com/apache/cordova-plugin-file/pull/405#discussion_r493143244



##########
File path: src/browser/FileProxy.js
##########
@@ -56,11 +57,6 @@
     var File = require('./File');
 
     (function (exports, global) {
-        var indexedDB = global.indexedDB || global.mozIndexedDB;

Review comment:
       Do not remove it in this PR.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org


[GitHub] [cordova-plugin-file] timbru31 commented on a change in pull request #405: refactor(eslint): use cordova-eslint /w fix

Posted by GitBox <gi...@apache.org>.
timbru31 commented on a change in pull request #405:
URL: https://github.com/apache/cordova-plugin-file/pull/405#discussion_r448953932



##########
File path: src/browser/FileProxy.js
##########
@@ -56,11 +57,6 @@
     var File = require('./File');
 
     (function (exports, global) {
-        var indexedDB = global.indexedDB || global.mozIndexedDB;

Review comment:
       Removed this check for Firefox( OS?). Caniuse is pretty green in terms of IndexedDB: https://caniuse.com/#feat=indexeddb




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org


[GitHub] [cordova-plugin-file] timbru31 commented on a change in pull request #405: refactor(eslint): use cordova-eslint /w fix

Posted by GitBox <gi...@apache.org>.
timbru31 commented on a change in pull request #405:
URL: https://github.com/apache/cordova-plugin-file/pull/405#discussion_r448952928



##########
File path: src/browser/FileProxy.js
##########
@@ -141,103 +147,121 @@
             // Create an absolute path if we were handed a relative one.
             path = resolveToFullPath_(fullPath, path);
 
-            idb_.get(path.storagePath, function (fileEntry) {
-                if (options.create === true && options.exclusive === true && fileEntry) {
-                    // If create and exclusive are both true, and the path already exists,
-                    // getFile must fail.
+            idb_.get(
+                path.storagePath,
+                function (fileEntry) {
+                    if (options.create === true && options.exclusive === true && fileEntry) {
+                        // If create and exclusive are both true, and the path already exists,
+                        // getFile must fail.
 
-                    if (errorCallback) {
-                        errorCallback(FileError.PATH_EXISTS_ERR);
-                    }
-                } else if (options.create === true && !fileEntry) {
-                    // If create is true, the path doesn't exist, and no other error occurs,
-                    // getFile must create it as a zero-length file and return a corresponding
-                    // FileEntry.
-                    var newFileEntry = new FileEntry(path.fileName, path.fullPath, new FileSystem(path.fsName, fs_.root));
-
-                    newFileEntry.file_ = new MyFile({
-                        size: 0,
-                        name: newFileEntry.name,
-                        lastModifiedDate: new Date(),
-                        storagePath: path.storagePath
-                    });
-
-                    idb_.put(newFileEntry, path.storagePath, successCallback, errorCallback);
-                } else if (options.create === true && fileEntry) {
-                    if (fileEntry.isFile) {
-                        // Overwrite file, delete then create new.
-                        idb_['delete'](path.storagePath, function () {
-                            var newFileEntry = new FileEntry(path.fileName, path.fullPath, new FileSystem(path.fsName, fs_.root));
-
-                            newFileEntry.file_ = new MyFile({
-                                size: 0,
-                                name: newFileEntry.name,
-                                lastModifiedDate: new Date(),
-                                storagePath: path.storagePath
-                            });
-
-                            idb_.put(newFileEntry, path.storagePath, successCallback, errorCallback);
-                        }, errorCallback);
-                    } else {
                         if (errorCallback) {
-                            errorCallback(FileError.INVALID_MODIFICATION_ERR);
+                            errorCallback(FileError.PATH_EXISTS_ERR);
                         }
-                    }
-                } else if ((!options.create || options.create === false) && !fileEntry) {
-                    // If create is not true and the path doesn't exist, getFile must fail.
-                    if (errorCallback) {
-                        errorCallback(FileError.NOT_FOUND_ERR);
-                    }
-                } else if ((!options.create || options.create === false) && fileEntry &&
-                    fileEntry.isDirectory) {
-                    // If create is not true and the path exists, but is a directory, getFile
-                    // must fail.
-                    if (errorCallback) {
-                        errorCallback(FileError.TYPE_MISMATCH_ERR);
-                    }
-                } else {
-                    // Otherwise, if no other error occurs, getFile must return a FileEntry
-                    // corresponding to path.
+                    } else if (options.create === true && !fileEntry) {
+                        // If create is true, the path doesn't exist, and no other error occurs,
+                        // getFile must create it as a zero-length file and return a corresponding
+                        // FileEntry.
+                        var newFileEntry = new FileEntry(path.fileName, path.fullPath, new FileSystem(path.fsName, fs_.root));
+
+                        newFileEntry.file_ = new MyFile({
+                            size: 0,
+                            name: newFileEntry.name,
+                            lastModifiedDate: new Date(),
+                            storagePath: path.storagePath
+                        });
+
+                        idb_.put(newFileEntry, path.storagePath, successCallback, errorCallback);
+                    } else if (options.create === true && fileEntry) {
+                        if (fileEntry.isFile) {
+                            // Overwrite file, delete then create new.
+                            idb_.delete(
+                                path.storagePath,
+                                function () {
+                                    var newFileEntry = new FileEntry(path.fileName, path.fullPath, new FileSystem(path.fsName, fs_.root));
+
+                                    newFileEntry.file_ = new MyFile({
+                                        size: 0,
+                                        name: newFileEntry.name,
+                                        lastModifiedDate: new Date(),
+                                        storagePath: path.storagePath
+                                    });
+
+                                    idb_.put(newFileEntry, path.storagePath, successCallback, errorCallback);
+                                },
+                                errorCallback
+                            );
+                        } else {
+                            if (errorCallback) {
+                                errorCallback(FileError.INVALID_MODIFICATION_ERR);
+                            }
+                        }
+                    } else if ((!options.create || options.create === false) && !fileEntry) {
+                        // If create is not true and the path doesn't exist, getFile must fail.
+                        if (errorCallback) {
+                            errorCallback(FileError.NOT_FOUND_ERR);
+                        }
+                    } else if ((!options.create || options.create === false) && fileEntry && fileEntry.isDirectory) {
+                        // If create is not true and the path exists, but is a directory, getFile
+                        // must fail.
+                        if (errorCallback) {
+                            errorCallback(FileError.TYPE_MISMATCH_ERR);
+                        }
+                    } else {
+                        // Otherwise, if no other error occurs, getFile must return a FileEntry
+                        // corresponding to path.
 
-                    successCallback(fileEntryFromIdbEntry(fileEntry));
-                }
-            }, errorCallback);
+                        successCallback(fileEntryFromIdbEntry(fileEntry));
+                    }
+                },
+                errorCallback
+            );
         };
 
         exports.getFileMetadata = function (successCallback, errorCallback, args) {
             var fullPath = args[0];
 
-            exports.getFile(function (fileEntry) {
-                successCallback(new File(fileEntry.file_.name, fileEntry.fullPath, '', fileEntry.file_.lastModifiedDate,
-                    fileEntry.file_.size));
-            }, errorCallback, [fullPath, null]);
+            exports.getFile(
+                function (fileEntry) {
+                    successCallback(
+                        new File(fileEntry.file_.name, fileEntry.fullPath, '', fileEntry.file_.lastModifiedDate, fileEntry.file_.size)
+                    );
+                },
+                errorCallback,
+                [fullPath, null]
+            );
         };
 
         exports.getMetadata = function (successCallback, errorCallback, args) {
-            exports.getFile(function (fileEntry) {
-                successCallback(
-                    {
+            exports.getFile(
+                function (fileEntry) {
+                    successCallback({
                         modificationTime: fileEntry.file_.lastModifiedDate,
                         size: fileEntry.file_.lastModifiedDate
                     });
-            }, errorCallback, args);
+                },
+                errorCallback,
+                args
+            );
         };
 
         exports.setMetadata = function (successCallback, errorCallback, args) {
             var fullPath = args[0];
             var metadataObject = args[1];
 
-            exports.getFile(function (fileEntry) {
-                fileEntry.file_.lastModifiedDate = metadataObject.modificationTime;
-                idb_.put(fileEntry, fileEntry.file_.storagePath, successCallback, errorCallback);
-            }, errorCallback, [fullPath, null]);
+            exports.getFile(
+                function (fileEntry) {
+                    fileEntry.file_.lastModifiedDate = metadataObject.modificationTime;
+                    idb_.put(fileEntry, fileEntry.file_.storagePath, successCallback, errorCallback);
+                },
+                errorCallback,
+                [fullPath, null]
+            );
         };
 
         exports.write = function (successCallback, errorCallback, args) {
             var fileName = args[0];
             var data = args[1];
             var position = args[2];
-            var isBinary = args[3]; // eslint-disable-line no-unused-vars

Review comment:
       No reason to allocate memory if the variable is not used.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org


[GitHub] [cordova-plugin-file] erisu commented on a change in pull request #405: refactor(eslint): use cordova-eslint /w fix

Posted by GitBox <gi...@apache.org>.
erisu commented on a change in pull request #405:
URL: https://github.com/apache/cordova-plugin-file/pull/405#discussion_r493142989



##########
File path: src/browser/FileProxy.js
##########
@@ -81,13 +77,10 @@
 
         var unicodeLastChar = 65535;
 
-    /** * Exported functionality ***/
+        /** * Exported functionality ***/
 
         exports.requestFileSystem = function (successCallback, errorCallback, args) {
             var type = args[0];
-            // Size is ignored since IDB filesystem size depends
-            // on browser implementation and can't be set up by user
-            var size = args[1]; // eslint-disable-line no-unused-vars

Review comment:
       remove it.

##########
File path: src/browser/FileProxy.js
##########
@@ -141,103 +147,121 @@
             // Create an absolute path if we were handed a relative one.
             path = resolveToFullPath_(fullPath, path);
 
-            idb_.get(path.storagePath, function (fileEntry) {
-                if (options.create === true && options.exclusive === true && fileEntry) {
-                    // If create and exclusive are both true, and the path already exists,
-                    // getFile must fail.
+            idb_.get(
+                path.storagePath,
+                function (fileEntry) {
+                    if (options.create === true && options.exclusive === true && fileEntry) {
+                        // If create and exclusive are both true, and the path already exists,
+                        // getFile must fail.
 
-                    if (errorCallback) {
-                        errorCallback(FileError.PATH_EXISTS_ERR);
-                    }
-                } else if (options.create === true && !fileEntry) {
-                    // If create is true, the path doesn't exist, and no other error occurs,
-                    // getFile must create it as a zero-length file and return a corresponding
-                    // FileEntry.
-                    var newFileEntry = new FileEntry(path.fileName, path.fullPath, new FileSystem(path.fsName, fs_.root));
-
-                    newFileEntry.file_ = new MyFile({
-                        size: 0,
-                        name: newFileEntry.name,
-                        lastModifiedDate: new Date(),
-                        storagePath: path.storagePath
-                    });
-
-                    idb_.put(newFileEntry, path.storagePath, successCallback, errorCallback);
-                } else if (options.create === true && fileEntry) {
-                    if (fileEntry.isFile) {
-                        // Overwrite file, delete then create new.
-                        idb_['delete'](path.storagePath, function () {
-                            var newFileEntry = new FileEntry(path.fileName, path.fullPath, new FileSystem(path.fsName, fs_.root));
-
-                            newFileEntry.file_ = new MyFile({
-                                size: 0,
-                                name: newFileEntry.name,
-                                lastModifiedDate: new Date(),
-                                storagePath: path.storagePath
-                            });
-
-                            idb_.put(newFileEntry, path.storagePath, successCallback, errorCallback);
-                        }, errorCallback);
-                    } else {
                         if (errorCallback) {
-                            errorCallback(FileError.INVALID_MODIFICATION_ERR);
+                            errorCallback(FileError.PATH_EXISTS_ERR);
                         }
-                    }
-                } else if ((!options.create || options.create === false) && !fileEntry) {
-                    // If create is not true and the path doesn't exist, getFile must fail.
-                    if (errorCallback) {
-                        errorCallback(FileError.NOT_FOUND_ERR);
-                    }
-                } else if ((!options.create || options.create === false) && fileEntry &&
-                    fileEntry.isDirectory) {
-                    // If create is not true and the path exists, but is a directory, getFile
-                    // must fail.
-                    if (errorCallback) {
-                        errorCallback(FileError.TYPE_MISMATCH_ERR);
-                    }
-                } else {
-                    // Otherwise, if no other error occurs, getFile must return a FileEntry
-                    // corresponding to path.
+                    } else if (options.create === true && !fileEntry) {
+                        // If create is true, the path doesn't exist, and no other error occurs,
+                        // getFile must create it as a zero-length file and return a corresponding
+                        // FileEntry.
+                        var newFileEntry = new FileEntry(path.fileName, path.fullPath, new FileSystem(path.fsName, fs_.root));
+
+                        newFileEntry.file_ = new MyFile({
+                            size: 0,
+                            name: newFileEntry.name,
+                            lastModifiedDate: new Date(),
+                            storagePath: path.storagePath
+                        });
+
+                        idb_.put(newFileEntry, path.storagePath, successCallback, errorCallback);
+                    } else if (options.create === true && fileEntry) {
+                        if (fileEntry.isFile) {
+                            // Overwrite file, delete then create new.
+                            idb_.delete(
+                                path.storagePath,
+                                function () {
+                                    var newFileEntry = new FileEntry(path.fileName, path.fullPath, new FileSystem(path.fsName, fs_.root));
+
+                                    newFileEntry.file_ = new MyFile({
+                                        size: 0,
+                                        name: newFileEntry.name,
+                                        lastModifiedDate: new Date(),
+                                        storagePath: path.storagePath
+                                    });
+
+                                    idb_.put(newFileEntry, path.storagePath, successCallback, errorCallback);
+                                },
+                                errorCallback
+                            );
+                        } else {
+                            if (errorCallback) {
+                                errorCallback(FileError.INVALID_MODIFICATION_ERR);
+                            }
+                        }
+                    } else if ((!options.create || options.create === false) && !fileEntry) {
+                        // If create is not true and the path doesn't exist, getFile must fail.
+                        if (errorCallback) {
+                            errorCallback(FileError.NOT_FOUND_ERR);
+                        }
+                    } else if ((!options.create || options.create === false) && fileEntry && fileEntry.isDirectory) {
+                        // If create is not true and the path exists, but is a directory, getFile
+                        // must fail.
+                        if (errorCallback) {
+                            errorCallback(FileError.TYPE_MISMATCH_ERR);
+                        }
+                    } else {
+                        // Otherwise, if no other error occurs, getFile must return a FileEntry
+                        // corresponding to path.
 
-                    successCallback(fileEntryFromIdbEntry(fileEntry));
-                }
-            }, errorCallback);
+                        successCallback(fileEntryFromIdbEntry(fileEntry));
+                    }
+                },
+                errorCallback
+            );
         };
 
         exports.getFileMetadata = function (successCallback, errorCallback, args) {
             var fullPath = args[0];
 
-            exports.getFile(function (fileEntry) {
-                successCallback(new File(fileEntry.file_.name, fileEntry.fullPath, '', fileEntry.file_.lastModifiedDate,
-                    fileEntry.file_.size));
-            }, errorCallback, [fullPath, null]);
+            exports.getFile(
+                function (fileEntry) {
+                    successCallback(
+                        new File(fileEntry.file_.name, fileEntry.fullPath, '', fileEntry.file_.lastModifiedDate, fileEntry.file_.size)
+                    );
+                },
+                errorCallback,
+                [fullPath, null]
+            );
         };
 
         exports.getMetadata = function (successCallback, errorCallback, args) {
-            exports.getFile(function (fileEntry) {
-                successCallback(
-                    {
+            exports.getFile(
+                function (fileEntry) {
+                    successCallback({
                         modificationTime: fileEntry.file_.lastModifiedDate,
                         size: fileEntry.file_.lastModifiedDate
                     });
-            }, errorCallback, args);
+                },
+                errorCallback,
+                args
+            );
         };
 
         exports.setMetadata = function (successCallback, errorCallback, args) {
             var fullPath = args[0];
             var metadataObject = args[1];
 
-            exports.getFile(function (fileEntry) {
-                fileEntry.file_.lastModifiedDate = metadataObject.modificationTime;
-                idb_.put(fileEntry, fileEntry.file_.storagePath, successCallback, errorCallback);
-            }, errorCallback, [fullPath, null]);
+            exports.getFile(
+                function (fileEntry) {
+                    fileEntry.file_.lastModifiedDate = metadataObject.modificationTime;
+                    idb_.put(fileEntry, fileEntry.file_.storagePath, successCallback, errorCallback);
+                },
+                errorCallback,
+                [fullPath, null]
+            );
         };
 
         exports.write = function (successCallback, errorCallback, args) {
             var fileName = args[0];
             var data = args[1];
             var position = args[2];
-            var isBinary = args[3]; // eslint-disable-line no-unused-vars

Review comment:
       remove it.

##########
File path: src/browser/FileProxy.js
##########
@@ -521,7 +581,7 @@
             }
 
             // support for encodeURI
-            if (/\%5/g.test(path) || /\%20/g.test(path)) {  // eslint-disable-line no-useless-escape
+            if (/%5/g.test(path) || /%20/g.test(path)) {

Review comment:
       remove it.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org


[GitHub] [cordova-plugin-file] erisu commented on a change in pull request #405: refactor(eslint): use cordova-eslint /w fix

Posted by GitBox <gi...@apache.org>.
erisu commented on a change in pull request #405:
URL: https://github.com/apache/cordova-plugin-file/pull/405#discussion_r493143602



##########
File path: www/android/FileSystem.js
##########
@@ -17,9 +17,11 @@
  * specific language governing permissions and limitations
  * under the License.
  *
-*/
+ */
 
-FILESYSTEM_PROTOCOL = 'cdvfile'; // eslint-disable-line no-undef

Review comment:
       Yes, add `var` and remove the `eslint-disable-line no-undef`




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org