Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Delta Between Two Patch Sets: lib/io.js

Issue 29433625: Issue 5220 - Update the IO API (Closed) Base URL: https://hg.adblockplus.org/adblockpluschrome/
Left Patch Set: Fix the IO implementation based on feedback. Created May 10, 2017, 4:08 p.m.
Right Patch Set: Fixes to the promise flow and review comments. Created May 23, 2017, 12:31 p.m.
Left:
Right:
Use n/p to move between diff chunks; N/P to move between comments.
Jump to:
Left: Side by side diff | Download
Right: Side by side diff | Download
« no previous file with change/comment | « lib/compat.js ('k') | no next file » | no next file with change/comment »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
LEFTRIGHT
1 /* 1 /*
2 * This file is part of Adblock Plus <https://adblockplus.org/>, 2 * This file is part of Adblock Plus <https://adblockplus.org/>,
3 * Copyright (C) 2006-2017 eyeo GmbH 3 * Copyright (C) 2006-2017 eyeo GmbH
4 * 4 *
5 * Adblock Plus is free software: you can redistribute it and/or modify 5 * Adblock Plus is free software: you can redistribute it and/or modify
6 * it under the terms of the GNU General Public License version 3 as 6 * it under the terms of the GNU General Public License version 3 as
7 * published by the Free Software Foundation. 7 * published by the Free Software Foundation.
8 * 8 *
9 * Adblock Plus is distributed in the hope that it will be useful, 9 * Adblock Plus is distributed in the hope that it will be useful,
10 * but WITHOUT ANY WARRANTY; without even the implied warranty of 10 * but WITHOUT ANY WARRANTY; without even the implied warranty of
(...skipping 43 matching lines...) Expand 10 before | Expand all | Expand 10 after
54 }, 54 },
55 resolve 55 resolve
56 ); 56 );
57 }); 57 });
58 } 58 }
59 59
60 function removeFile(fileName) 60 function removeFile(fileName)
61 { 61 {
62 return new Promise((resolve, reject) => 62 return new Promise((resolve, reject) =>
63 { 63 {
64 ext.storage.remove(fileToKey(fileName), resolve); 64 ext.storage.remove(fileToKey(fileName), () =>
65 {
66 if (chrome.runtime.lastError)
67 reject(chrome.runtime.lastError.message);
68 else
69 resolve();
70 });
65 }); 71 });
66 } 72 }
67 73
68 exports.IO = 74 exports.IO =
69 { 75 {
70 /** 76 /**
71 * Reads text lines from a file. 77 * Reads text lines from a file.
72 * @param {string} fileName 78 * @param {string} fileName
73 * Name of the file to be read 79 * Name of the file to be read
Sebastian Noack 2017/05/22 13:36:30 Nit: This is inconsistent with how we wrap documen
Wladimir Palant 2017/05/22 13:46:31 Yes, I am aware of that - and wrapping it this way
Sebastian Noack 2017/05/22 13:49:52 Adding Dave for a third opinion.
kzar 2017/05/22 14:00:41 Well IIRC I've done it both ways depending on the
hub 2017/05/23 12:31:50 I copied (and proofed) these documentation comment
74 * @param {TextSink} listener 80 * @param {TextSink} listener
75 * Function that will be called for each line in the file 81 * Function that will be called for each line in the file
76 * @return {Promise} 82 * @return {Promise}
77 * Promise to be resolved or rejected once the operation is completed 83 * Promise to be resolved or rejected once the operation is completed
78 */ 84 */
79 readFromFile(fileName, listener) 85 readFromFile(fileName, listener)
80 { 86 {
81 return loadFile(fileName).then(entry => 87 return loadFile(fileName).then(entry =>
82 { 88 {
83 for (let line of entry.content) 89 for (let line of entry.content)
84 listener(line); 90 listener(line);
Sebastian Noack 2017/05/22 13:36:30 Nit: Please use 2 (not 4) space indentation. Howev
Wladimir Palant 2017/05/22 13:46:31 This comment doesn't appear to match the line it h
Sebastian Noack 2017/05/22 13:49:52 Sorry, I misread the code. For some reason I didn'
85 }); 91 });
86 }, 92 },
87 93
88 /** 94 /**
89 * Writes text lines to a file. 95 * Writes text lines to a file.
90 * @param {string} fileName 96 * @param {string} fileName
91 * Name of the file to be written 97 * Name of the file to be written
92 * @param {Iterable.<string>} data 98 * @param {Iterable.<string>} data
93 * An array-like or iterable object containing the lines (without line 99 * An array-like or iterable object containing the lines (without line
94 * endings) 100 * endings)
95 * @return {Promise} 101 * @return {Promise}
96 * Promise to be resolved or rejected once the operation is completed 102 * Promise to be resolved or rejected once the operation is completed
97 */ 103 */
98 writeToFile(fileName, data) 104 writeToFile(fileName, data)
99 { 105 {
100 return saveFile(fileName, data); 106 return saveFile(fileName, data);
101 }, 107 },
102 108
103 /** 109 /**
104 * Copies a file. 110 * Copies a file.
105 * @param {string} fromFile 111 * @param {string} fromFile
106 * Name of the file to be copied 112 * Name of the file to be copied
107 * @param {string} toFile 113 * @param {string} toFile
108 * Name of the file to be written, will be overwritten if exists 114 * Name of the file to be written, will be overwritten if exists
109 * @return {Promise} 115 * @return {Promise}
110 * Promise to be resolved or rejected once the operation is completed 116 * Promise to be resolved or rejected once the operation is completed
111 */ 117 */
112 copyFile(fromFile, toFile) 118 copyFile(fromFile, toFile)
113 { 119 {
114 return loadFile(fromFile).then(entry => 120 return loadFile(fromFile).then(entry => saveFile(toFile, entry.content));
115 {
116 return saveFile(toFile, entry.content).then(resolve());
117 });
118 }, 121 },
119 122
120 /** 123 /**
121 * Renames a file. 124 * Renames a file.
122 * @param {string} fromFile 125 * @param {string} fromFile
123 * Name of the file to be renamed 126 * Name of the file to be renamed
124 * @param {string} newName 127 * @param {string} newName
125 * New file name, will be overwritten if exists 128 * New file name, will be overwritten if exists
126 * @return {Promise} 129 * @return {Promise}
127 * Promise to be resolved or rejected once the operation is completed 130 * Promise to be resolved or rejected once the operation is completed
128 */ 131 */
129 renameFile(fromFile, newName) 132 renameFile(fromFile, newName)
130 { 133 {
131 return loadFile(fromFile).then(entry => 134 return loadFile(fromFile).then(entry =>
132 { 135 {
133 ext.storage.set(fileToKey(newName), entry, resolve); 136 return new Promise((resolve, reject) =>
134 }).then(() => 137 {
135 { 138 ext.storage.set(fileToKey(newName), entry, () =>
Sebastian Noack 2017/05/22 13:36:30 The parentheses + return is redundant here.
hub 2017/05/23 12:31:50 Done.
136 return removeFile(fromFile); 139 {
137 }); 140 if (chrome.runtime.lastError)
141 reject(chrome.runtime.lastError.message);
142 else
143 resolve();
144 });
145 });
146 }).then(() => removeFile(fromFile));
138 }, 147 },
139 148
140 /** 149 /**
141 * Removes a file. 150 * Removes a file.
142 * @param {string} fileName 151 * @param {string} fileName
143 * Name of the file to be removed 152 * Name of the file to be removed
144 * @return {Promise} 153 * @return {Promise}
145 * Promise to be resolved or rejected once the operation is completed 154 * Promise to be resolved or rejected once the operation is completed
146 */ 155 */
147 removeFile(fileName) 156 removeFile(fileName)
(...skipping 18 matching lines...) Expand all
166 lastModified: entry.lastModified 175 lastModified: entry.lastModified
167 }; 176 };
168 }).catch(error => 177 }).catch(error =>
169 { 178 {
170 if (error.type == "NoSuchFile") 179 if (error.type == "NoSuchFile")
171 return {exists: false}; 180 return {exists: false};
172 throw error; 181 throw error;
173 }); 182 });
174 } 183 }
175 }; 184 };
LEFTRIGHT

Powered by Google App Engine
This is Rietveld