From b148140c58e90fa3c72e50dab39abfecdff73318 Mon Sep 17 00:00:00 2001 From: Quentin Rossetti Date: Fri, 24 Oct 2014 01:31:57 +0200 Subject: [PATCH] RegExp for parsing and switches passed to `run` #2 In order to clean up code and be more aware of spaces, the command (ie: non-switches parameters) are parsed with a RegExp so they understand paths even when they contain spaces (with the help of double quotes). Also switches are converted into an array (`child_process.spawn` compatible) and passed to the `run` function in each action. This avoid an useless stringify/parse of the switches. --- TOTO.md | 7 +++ lib/add.js | 8 +--- lib/delete.js | 12 +---- lib/extract.js | 7 +-- lib/extractFull.js | 8 +--- lib/list.js | 5 +-- lib/test.js | 11 +---- lib/update.js | 12 +---- test/method.Zip.add.js | 8 ++-- test/method.Zip.extract.js | 14 +++--- test/method.Zip.extractFull.js | 4 +- test/method.Zip.list.js | 10 ++--- test/util.run.js | 47 ++++++++++++++++++- test/util.switches.js | 48 +++++++++++++------- util/run.js | 82 +++++++++++++++++++++------------- util/switches.js | 42 ++++++++++------- 16 files changed, 194 insertions(+), 131 deletions(-) create mode 100644 TOTO.md diff --git a/TOTO.md b/TOTO.md new file mode 100644 index 0000000..75a0069 --- /dev/null +++ b/TOTO.md @@ -0,0 +1,7 @@ +TODO +==== + + - [x] Pass switches from each command to the `run` function. + - [x] Normalize path in `run`? + - [ ] Streaming abilities + - [ ] Multiple files in add, delete, etc. diff --git a/lib/add.js b/lib/add.js index 4b1ccf2..8795a8d 100644 --- a/lib/add.js +++ b/lib/add.js @@ -22,13 +22,9 @@ module.exports = function (archive, files, options) { options = {}; } - archive = archive.replace(/\ /g, '\\ '); - files = files.replace(/\ /g, '\\ '); + var command = '7za a "' + archive + '" "' + files + '"'; - var opts = u.switches(options); - var command = '7za a ' + archive + ' ' + files + ' ' + opts; - - u.run(command) + u.run(command, options) // When a stdout is emitted, parse each line and search for a pattern. When // the pattern is found, extract the file (or directory) name from it and diff --git a/lib/delete.js b/lib/delete.js index 4bfe0f6..3ab75d9 100644 --- a/lib/delete.js +++ b/lib/delete.js @@ -18,16 +18,8 @@ var u = { module.exports = function (archive, files, options) { return when.promise(function (fulfill, reject, progress) { - if (options === undefined) { - options = {}; - } - - archive = archive.replace(/\ /g, '\\ '); - files = files.replace(/\ /g, '\\ '); - - var opts = u.switches(options); - var command = '7za d ' + archive + ' ' + files + ' ' + opts; - u.run(command) + var command = '7za d "' + archive + '" "' + files + '"'; + u.run(command, options) .then(function () { fulfill(); diff --git a/lib/extract.js b/lib/extract.js index 16a7740..f8afd5d 100644 --- a/lib/extract.js +++ b/lib/extract.js @@ -22,11 +22,8 @@ module.exports = function (archive, dest, options) { options = {}; } - archive = archive.replace(/\ /g, '\\ '); - dest = dest.replace(/\ /g, '\\ '); - var opts = u.switches(options); - var command = '7za e ' + archive + ' -o' + dest + ' ' + opts; - u.run(command) + var command = '7za e "' + archive + '" -o"' + dest + '" '; + u.run(command, options) // When a stdout is emitted, parse each line and search for a pattern. When // the pattern is found, extract the file (or directory) name from it and diff --git a/lib/extractFull.js b/lib/extractFull.js index aef71c7..0f1cff3 100644 --- a/lib/extractFull.js +++ b/lib/extractFull.js @@ -22,13 +22,9 @@ module.exports = function (archive, dest, options) { options = {}; } - dest = dest.replace(/\ /g, '\\ '); - archive = archive.replace(/\ /g, '\\ '); + var command = '7za x "' + archive + '" -o"' + dest + '" '; - var opts = u.switches(options); - var command = '7za x ' + archive + ' -o' + dest + ' ' + opts; - - u.run(command) + u.run(command, options) // When a stdout is emitted, parse each line and search for a pattern. When // the pattern is found, extract the file (or directory) name from it and diff --git a/lib/list.js b/lib/list.js index 36ce571..5d94205 100644 --- a/lib/list.js +++ b/lib/list.js @@ -24,11 +24,10 @@ module.exports = function (archive, options) { archive = archive.replace(/\ /g, '\\ '); - var opts = u.switches(options); var spec = {}; var regex = /(\d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2}) ([\.DA]+) +(\d+)[ \d]+ (.+)/; - var command = '7za l ' + archive + ' ' + opts; - u.run(command) + var command = '7za l "' + archive + '" '; + u.run(command, options) // When a stdout is emitted, parse each line and search for a pattern. When // the pattern is found, extract the file (or directory) name from it and diff --git a/lib/test.js b/lib/test.js index bf1c8c5..68fd03c 100644 --- a/lib/test.js +++ b/lib/test.js @@ -17,15 +17,8 @@ var u = { module.exports = function (archive, options) { return when.promise(function (fulfill, reject, progress) { - if (options === undefined) { - options = {}; - } - - archive = archive.replace(/\ /g, '\\ '); - - var opts = u.switches(options); - var command = '7za t ' + archive + ' ' + opts; - u.run(command) + var command = '7za t "' + archive + '"'; + u.run(command, options) // When a stdout is emitted, parse each line and search for a pattern. When // the pattern is found, extract the file (or directory) name from it and diff --git a/lib/update.js b/lib/update.js index 26d9682..e008c22 100644 --- a/lib/update.js +++ b/lib/update.js @@ -18,16 +18,8 @@ var u = { module.exports = function (archive, files, options) { return when.promise(function (fulfill, reject, progress) { - if (options === undefined) { - options = {}; - } - - archive = archive.replace(/\ /g, '\\ '); - files = files.replace(/\ /g, '\\ '); - - var opts = u.switches(options); - var command = '7za u ' + archive + ' ' + files + ' ' + opts; - u.run(command) + var command = '7za u "' + archive + '" "' + files + '"'; + u.run(command, options) // When a stdout is emitted, parse each line and search for a pattern. When // the pattern is found, extract the file (or directory) name from it and diff --git a/test/method.Zip.add.js b/test/method.Zip.add.js index 45e2d1b..49a10b1 100644 --- a/test/method.Zip.add.js +++ b/test/method.Zip.add.js @@ -4,9 +4,9 @@ var fs = require('fs-extra'); var add = require('../lib/add'); describe('Method: `Zip.add`', function () { - + afterEach(function () { fs.removeSync('.tmp/test'); }); - + it('should return an error on 7z error', function (done) { add('.tmp/test/addnot.7z', '.tmp/test/nothere', { '???': true }) .catch(function (err) { @@ -14,7 +14,7 @@ describe('Method: `Zip.add`', function () { done(); }); }); - + it('should return entries on progress', function (done) { add('.tmp/test/add.zip', '*.md') .progress(function (entries) { @@ -22,5 +22,5 @@ describe('Method: `Zip.add`', function () { done(); }); }); - + }); diff --git a/test/method.Zip.extract.js b/test/method.Zip.extract.js index 4207bc2..fb7aa79 100644 --- a/test/method.Zip.extract.js +++ b/test/method.Zip.extract.js @@ -4,9 +4,9 @@ var fs = require('fs-extra'); var extract = require('../lib/extract'); describe('Method: `Zip.extract`', function () { - - afterEach(function () { fs.removeSync('.tmp/test'); }); - + + //afterEach(function () { fs.removeSync('.tmp/test'); }); + it('should return an error on 7z error', function (done) { extract('test/nothere.7z', '.tmp/test') .catch(function (err) { @@ -22,7 +22,7 @@ describe('Method: `Zip.extract`', function () { done(); }); }); - + it('should return entries on progress', function (done) { extract('test/zip.7z', '.tmp/test') .progress(function (entries) { @@ -30,7 +30,7 @@ describe('Method: `Zip.extract`', function () { done(); }); }); - + it('should extract on the right path', function (done) { extract('test/zip.7z', '.tmp/test') .then(function () { @@ -41,5 +41,5 @@ describe('Method: `Zip.extract`', function () { done(); }); }); - -}); \ No newline at end of file + +}); diff --git a/test/method.Zip.extractFull.js b/test/method.Zip.extractFull.js index 55be766..17e02b0 100644 --- a/test/method.Zip.extractFull.js +++ b/test/method.Zip.extractFull.js @@ -15,7 +15,7 @@ describe('Method: `Zip.extractFull`', function () { }); }); - it('should return an error on output duplticate', function (done) { + it('should return an error on output duplicate', function (done) { extractFull('test/zip.7z', '.tmp/test', { o: '.tmp/test/duplicate' }) .catch(function (err) { expect(err).to.be.an.instanceof(Error); @@ -54,7 +54,7 @@ describe('Method: `Zip.extractFull`', function () { done(); }); }); - + it('should work with spaces in both source and destination', function (done) { fs.copySync('test/zip.7z','.tmp/test/Folder From/Folder A/Folder B/Folder C/zip file.7z'); extractFull('.tmp/test/Folder From/Folder A/Folder B/Folder C/zip file.7z','.tmp/test/Folder To/Folder D/Folder E/Folder F') diff --git a/test/method.Zip.list.js b/test/method.Zip.list.js index d34d918..f3289ee 100644 --- a/test/method.Zip.list.js +++ b/test/method.Zip.list.js @@ -3,7 +3,7 @@ var expect = require('chai').expect; var list = require('../lib/list'); describe('Method: `Zip.list`', function () { - + it('should return an error on 7z error', function (done) { list('test/nothere.7z') .catch(function (err) { @@ -11,7 +11,7 @@ describe('Method: `Zip.list`', function () { done(); }); }); - + it('should return an tech spec on fulfill', function (done) { list('test/zip.7z', { r: true }) .then(function (spec) { @@ -23,7 +23,7 @@ describe('Method: `Zip.list`', function () { done(); }); }); - + it('should return valid entries on progress', function (done) { list('test/zip.zip') .progress(function (entries) { @@ -35,5 +35,5 @@ describe('Method: `Zip.list`', function () { done(); }); }); - -}); \ No newline at end of file + +}); diff --git a/test/util.run.js b/test/util.run.js index 0e6ad3c..4bd271d 100644 --- a/test/util.run.js +++ b/test/util.run.js @@ -12,14 +12,14 @@ describe('Utility: `run`', function () { }); it('should return an error on when 7z gets one', function (done) { - run('7za ?').catch(function (err) { + run('7za "???"').catch(function (err) { expect(err.message).to.eql('Incorrect command line'); done(); }); }); it('should return an stdout on progress', function (done) { - run('7za') + run('7za', { h: true }) .progress(function (data) { expect(data).to.be.a('string'); }) @@ -28,4 +28,47 @@ describe('Utility: `run`', function () { }); }); + it('should correctly parse complex commands', function (done) { + run('7za a ".tmp/test/archive.7z" "*.exe" "*.dll"', { + m0: '=BCJ', + m1: '=LZMA:d=21' + }) + .then(function (res) { + expect(res.args).to.eql([ + 'a', + '.tmp/test/archive.7z', + '*.exe', + '*.dll', + '-m0=BCJ', + '-m1=LZMA:d=21', + '-ssc', + '-y', + ]); + done(); + }); + }); + + it('should correctly parse complex commands with spaces', function (done) { + var sep = require('path').sep; + run('7za a ".tmp/Folder A/Folder B\\archive.7z" "*.exe" "*.dll"', { + m0: '=BCJ', + m1: '=LZMA:d=21', + p : 'My mhjls/\\c $^é5°', + }) + .then(function (res) { + expect(res.args).to.eql([ + 'a', + '.tmp/Folder A'+sep+'Folder B'+sep+'archive.7z', + '*.exe', + '*.dll', + '-m0=BCJ', + '-m1=LZMA:d=21', + '-p"My mhjls/\\c $^é5°"', + '-ssc', + '-y' + ]); + done(); + }); + }); + }); diff --git a/test/util.switches.js b/test/util.switches.js index ee17b01..9297771 100644 --- a/test/util.switches.js +++ b/test/util.switches.js @@ -3,43 +3,61 @@ var expect = require('chai').expect; var switches = require('../util/switches'); describe('Utility: `switches`', function () { - + it('should return deflaut flags with no args', function () { - expect(switches({})).to.eql('-ssc -y'); - }); - + expect(switches({})).to.contain('-ssc'); + expect(switches({})).to.contain('-y'); + }); + it('should return -ssc with flag { ssc: true }', function () { - expect(switches({ ssc: true })).to.eql('-ssc -y'); - }); - + expect(switches({ ssc: true })).to.contain('-ssc'); + expect(switches({ ssc: true })).to.contain('-y'); + }); + it('should return -ssc- with flag { ssc: false }', function () { - expect(switches({ ssc: false })).to.eql('-ssc- -y'); + expect(switches({ ssc: false })).to.contain('-ssc-'); }); - + it('should return non default booleans when specified', function () { var r = switches({ so : true, spl: true, ssw: true, y : false - }); + }); expect(r).to.contain('-so'); expect(r).to.contain('-spl'); expect(r).to.contain('-ssc'); expect(r).to.contain('-ssw'); expect(r).not.to.contain('-y'); }); - + it('should return complex values when needed', function () { var r = switches({ ssc : true, ssw : true, - m : '0=BCJ -m1=LZMA:d=21' + mx0 : true + }); + expect(r).to.contain('-ssc'); + expect(r).to.contain('-ssw'); + expect(r).to.contain('-mx0'); + expect(r).to.contain('-y'); + }); + + it('should return complex values with spaces and quotes', function () { + var r = switches({ + ssc : true, + ssw : true, + m0 : '=BCJ', + m1 : '=LZMA:d=21', + p : 'My Super Pasw,àù£*', }); expect(r).to.contain('-ssc'); expect(r).to.contain('-ssw'); - expect(r).to.contain('m0=BCJ -m1=LZMA:d=21'); + expect(r).to.contain('-m0=BCJ'); + expect(r).to.contain('-m1=LZMA:d=21'); + expect(r).to.contain('-p"My Super Pasw,àù£*"'); expect(r).to.contain('-y'); }); - -}); \ No newline at end of file + +}); diff --git a/util/run.js b/util/run.js index 332731b..38edb10 100644 --- a/util/run.js +++ b/util/run.js @@ -3,15 +3,17 @@ var os = require('os'); var spawn = require('win-spawn'); var when = require('when'); var path = require('path'); +var utilSwitches = require('./switches'); /** * @promise Run * @param {string} command The command to run. + * @param {Array} switches Options for 7-Zip as an array. * @progress {string} stdout message. * @reject {Error} The error issued by 7-Zip. * @reject {number} Exit code issued by 7-Zip. */ -module.exports = function (command) { +module.exports = function (command, switches) { return when.promise(function (fulfill, reject, progress) { // Parse the command variable. If the command is not a string reject the @@ -20,43 +22,61 @@ module.exports = function (command) { if (typeof command !== 'string') { return reject(new Error('Command must be a string')); } - var args = command.split(' '); - var cmd = args[0]; - args.shift(); + var cmd = command.split(' ')[0]; + var args = [ command.split(' ')[1] ]; - // Recover pathes when the have spaces. The split process above didn't care - // if the string contains pathes with spaces. The loop bellow look for each - // item to have a escape char at the end, if is present concat with the next - // item to restore the original path. - var filterSpaces = function (elem, index, array) { - if (elem[elem.length - 1] === '\\') { - var firstPart = elem.substr(0, elem.length - 1); - var separator = ' '; - var lastPart = args[index + 1]; - args[index] = firstPart + separator + lastPart; - args.splice(index + 1, 1); - } - }; - - // Run the filter for each space. By splicing the array in the function - // above the filter does not run on the item just after one that is being - // removed. - for (var i = 0; i < args.length; i++) { - args.forEach(filterSpaces); + // Parse and add command (non-switches parameters) to `args`. + var regexpCommands = /"((?:\\.|[^"\\])*)"/g; + var commands = command.match(regexpCommands); + if (commands) { + commands.forEach(function (c) { + c = c.replace(/\//, path.sep); + c = c.replace(/\\/, path.sep); + c = path.normalize(c); + args.push(c) + }); } - - // Normalize pathes before passing them to 7-Zip. - if (args.length > 1) { - args[1] = path.normalize(args[1]); - args[2] = path.normalize(args[2]); + + // Special treatment for the output switch because it is exposed as a + // parameter in the API and not as a option. + var regexpOutput = /-o"((?:\\.|[^"\\])*)"/g; + var output = command.match(regexpOutput); + if (output) { + args.pop(); + var o = output[0]; + o = o.replace(/\//, path.sep); + o = o.replace(/\\/, path.sep); + o = o.replace(/"/g, ''); + o = path.normalize(o); + args.push(o) } - + + // Add switches to the `args` array. + var switchesArray = utilSwitches(switches); + switchesArray.forEach(function (s) { args.push(s) }); + + // Remove now double quotes. If present in the spawned process 7-Zip will + // read them as part of the paths (e.g.: create a `"archive.7z"` with + // quotes in the file-name); + args.forEach(function (e, i) { + if (typeof e !== 'string') return; + if (e.substr(0, 1) !== '-') { + e = e.replace(/^"/, ''); + e = e.replace(/"$/, ''); + args[i] = e; + } + }); + // When an stdout is emitted, parse it. If an error is detected in the body // of the stdout create an new error with the 7-Zip error message as the // error's message. Otherwise progress with stdout message. var err; var reg = new RegExp('Error:' + os.EOL + '?(.*)', 'g'); - var run = spawn(cmd, args, { stdio: 'pipe' }); + var res = { + cmd: cmd, + args: args, + options: { stdio: 'pipe' } }; + var run = spawn(res.cmd, res.args, res.options); run.stdout.on('data', function (data) { var res = reg.exec(data.toString()); if (res) { @@ -65,7 +85,7 @@ module.exports = function (command) { progress(data.toString()); }); run.on('close', function (code) { - if (code === 0) return fulfill(); + if (code === 0) return fulfill(res); reject(err, code); }); diff --git a/util/switches.js b/util/switches.js index 4d4eb1b..25ea4a9 100644 --- a/util/switches.js +++ b/util/switches.js @@ -1,33 +1,43 @@ 'use strict'; module.exports = function (switches) { - - var r = ''; + + if (!switches) switches = {}; + + var a = []; // Set defalut values of boolean switches switches.so = (switches.so === true) ? true : false; switches.spl = (switches.spl === true) ? true : false; switches.ssc = (switches.ssc === false) ? false : true ; switches.ssw = (switches.ssw === true) ? true : false; switches.y = (switches.y === false) ? false : true ; - + for (var s in switches) { - var concat = ''; - - if (switches[s] === true) { - concat += '-' + s + ' '; + + // Switches that are set or not. Just add them to the array if they are + // present. Differ the `ssc` switch treatment to later in the function. + if (switches[s] === true && s !== 'ssc') { + a.push('-' + s); } + + // Switches with a value. Detect if the value contains a space. If it does + // wrap the value with double quotes. Else just add the switch and its value + // to the string. Doubles quotes are used for parsing with a RegExp later. if (typeof switches[s] !== 'boolean') { - concat += '-' + s + switches[s] + ' '; + if (switches[s].indexOf(' ') === -1) { + a.push('-' + s + switches[s]); + } else { + a.push('-' + s + '"' + switches[s] + '"'); + } } - + // Special treatment for `-ssc` if (s === 'ssc') { - concat = (switches.ssc === true) ? '-ssc ' : '-ssc- '; + a.push((switches.ssc === true) ? '-ssc' : '-ssc-'); } - - r += concat; + } - - return r.trim(); - -}; \ No newline at end of file + + return a; + +};