From 6c48af30d1579072b0cd514807cfec4d5f4bb748 Mon Sep 17 00:00:00 2001 From: Danny McClanahan Date: Thu, 16 Apr 2015 03:23:26 -0500 Subject: [PATCH 01/36] Add #! support for executable scripts on Linux. Pass arguments to executable script unchanged if using "#!/usr/bin/env coffee". (Previously, "./test.coffee -abck" would be turned into "-a -b -c -k", for example.) Fixes #1752. --- src/optparse.coffee | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/optparse.coffee b/src/optparse.coffee index b89d20e072..43e8ff0acf 100644 --- a/src/optparse.coffee +++ b/src/optparse.coffee @@ -1,4 +1,4 @@ -{repeat} = require './helpers' +{repeat, isCoffee} = require './helpers' # A simple **OptionParser** class to parse option flags from the command-line. # Use it like so: @@ -25,6 +25,14 @@ exports.OptionParser = class OptionParser # parsers that allow you to attach callback actions for every flag. Instead, # you're responsible for interpreting the options object. parse: (args) -> + # Pass all arguments to script unchanged if first argument is the script to + # be run; assume no options are for the coffeescript compiler. This allows + # the use of '#!/usr/bin/env coffee' to run executable scripts on Linux + # systems, which do not parse the '--' argument in the first line correctly. + if (args.indexOf '--' is -1) and + (args.length > 0) and + (isCoffee args[0]) + return arguments: args options = arguments: [] skippingArgument = no originalArgs = args From 221dfc4b7a0eee989bb00f63638b45d8dc1f8679 Mon Sep 17 00:00:00 2001 From: Danny McClanahan Date: Wed, 19 Apr 2017 20:16:31 -0500 Subject: [PATCH 02/36] refactor option parsing clean up parsing code and in the process fix oustanding bug where coffeescript modified arguments meant for an executable script --- src/optparse.coffee | 89 ++++++++++++++++++++------------------------- 1 file changed, 39 insertions(+), 50 deletions(-) diff --git a/src/optparse.coffee b/src/optparse.coffee index 43e8ff0acf..3af583b250 100644 --- a/src/optparse.coffee +++ b/src/optparse.coffee @@ -1,4 +1,4 @@ -{repeat, isCoffee} = require './helpers' +{repeat} = require './helpers' # A simple **OptionParser** class to parse option flags from the command-line. # Use it like so: @@ -25,45 +25,26 @@ exports.OptionParser = class OptionParser # parsers that allow you to attach callback actions for every flag. Instead, # you're responsible for interpreting the options object. parse: (args) -> - # Pass all arguments to script unchanged if first argument is the script to - # be run; assume no options are for the coffeescript compiler. This allows - # the use of '#!/usr/bin/env coffee' to run executable scripts on Linux - # systems, which do not parse the '--' argument in the first line correctly. - if (args.indexOf '--' is -1) and - (args.length > 0) and - (isCoffee args[0]) - return arguments: args - options = arguments: [] - skippingArgument = no - originalArgs = args - args = normalizeArguments args - for arg, i in args - if skippingArgument - skippingArgument = no - continue - if arg is '--' - pos = originalArgs.indexOf '--' - options.arguments = options.arguments.concat originalArgs[(pos + 1)..] + state = + argsLeft: args[..] + options: {} + while (arg = state.argsLeft.shift())? + if (arg.match(LONG_FLAG) ? arg.match(SHORT_FLAG))? + tryMatchOptionalArgument(arg, state, @rules) + else if (multiMatch = arg.match(MULTI_FLAG))? + # Normalize arguments by expanding merged flags into multiple + # flags. This allows you to have `-wl` be the same as `--watch --lint`. + normalized = "-#{multiArg}" for multiArg in multiMatch[1].split '' + state.argsLeft.unshift(normalized...) + else + # the CS option parser is a little odd; options after the first + # non-option argument are treated as non-option arguments themselves. + # executable scripts do not need to have a `--` at the end of the + # shebang ("#!") line, and if they do, they won't work on Linux + state.argsLeft.unshift(arg) unless arg is '--' break - isOption = !!(arg.match(LONG_FLAG) or arg.match(SHORT_FLAG)) - # the CS option parser is a little odd; options after the first - # non-option argument are treated as non-option arguments themselves - seenNonOptionArg = options.arguments.length > 0 - unless seenNonOptionArg - matchedRule = no - for rule in @rules - if rule.shortFlag is arg or rule.longFlag is arg - value = true - if rule.hasArgument - skippingArgument = yes - value = args[i + 1] - options[rule.name] = if rule.isList then (options[rule.name] or []).concat value else value - matchedRule = yes - break - throw new Error "unrecognized option: #{arg}" if isOption and not matchedRule - if seenNonOptionArg or not isOption - options.arguments.push arg - options + state.options.arguments = state.argsLeft[..] + state.options # Return the help text for this **OptionParser**, listing and describing all # of the valid options, for `--help` and such. @@ -107,14 +88,22 @@ buildRule = (shortFlag, longFlag, description, options = {}) -> isList: !!(match and match[2]) } -# Normalize arguments by expanding merged flags into multiple flags. This allows -# you to have `-wl` be the same as `--watch --lint`. -normalizeArguments = (args) -> - args = args[..] - result = [] - for arg in args - if match = arg.match MULTI_FLAG - result.push '-' + l for l in match[1].split '' - else - result.push arg - result +addArgument = (rule, options, value) -> + options[rule.name] = if rule.isList + (options[rule.name] ? []).concat value + else value + +tryMatchOptionalArgument = (arg, state, rules) -> + for rule in rules + if arg in [rule.shortFlag, rule.longFlag] + if rule.hasArgument + value = state.argsLeft.shift() + if not value? + throw new Error "#{arg} requires a value, but was the last argument" + else + value = true + + addArgument(rule, state.options, value) + return + + throw new Error "unrecognized option: #{arg}" From 988f2af47a191bd41a43b97b8d90069cb8dd9b97 Mon Sep 17 00:00:00 2001 From: Danny McClanahan Date: Thu, 20 Apr 2017 05:19:58 -0500 Subject: [PATCH 03/36] address comments --- src/optparse.coffee | 105 ++++++++++++++++++++++++++------------------ 1 file changed, 62 insertions(+), 43 deletions(-) diff --git a/src/optparse.coffee b/src/optparse.coffee index 3af583b250..d008d5df23 100644 --- a/src/optparse.coffee +++ b/src/optparse.coffee @@ -15,8 +15,8 @@ exports.OptionParser = class OptionParser # [short-flag, long-flag, description] # # Along with an an optional banner for the usage help. - constructor: (rules, @banner) -> - @rules = buildRules rules + constructor: (ruleDecls, @banner) -> + @rules = buildRules ruleDecls # Parse the list of arguments, populating an `options` object with all of the # specified options, and return it. Options after the first non-option @@ -25,26 +25,42 @@ exports.OptionParser = class OptionParser # parsers that allow you to attach callback actions for every flag. Instead, # you're responsible for interpreting the options object. parse: (args) -> - state = - argsLeft: args[..] - options: {} - while (arg = state.argsLeft.shift())? - if (arg.match(LONG_FLAG) ? arg.match(SHORT_FLAG))? - tryMatchOptionalArgument(arg, state, @rules) - else if (multiMatch = arg.match(MULTI_FLAG))? - # Normalize arguments by expanding merged flags into multiple - # flags. This allows you to have `-wl` be the same as `--watch --lint`. - normalized = "-#{multiArg}" for multiArg in multiMatch[1].split '' - state.argsLeft.unshift(normalized...) - else - # the CS option parser is a little odd; options after the first - # non-option argument are treated as non-option arguments themselves. - # executable scripts do not need to have a `--` at the end of the - # shebang ("#!") line, and if they do, they won't work on Linux - state.argsLeft.unshift(arg) unless arg is '--' + + options = arguments: [] + for cmdLineArg, i in args + # The CS option parser is a little odd; options after the first + # non-option argument are treated as non-option arguments themselves. + # Executable scripts do not need to have a `--` at the end of the + # shebang ("#!") line, and if they do, they won't work on Linux. + multi = trySplitMultiFlag(cmdLineArg) + toProcess = multi or trySingleFlag(cmdLineArg) + # If the current argument could not be parsed as one or more arguments. + unless toProcess? + ++i if cmdLineArg is '--' + options.arguments = args[i..] break - state.options.arguments = state.argsLeft[..] - state.options + + # Normalize arguments by expanding merged flags into multiple + # flags. This allows you to have `-wl` be the same as `--watch --lint`. + for argFlag, j in toProcess + rule = @rules.flagDict[argFlag] + unless rule? + context = if multi? then " (in multi-flag '#{cmdLineArg}')" else '' + throw new Error "unrecognized option: #{argFlag}#{context}" + + {hasArgument, isList, name} = rule + + options[name] = switch hasArgument + when no then true + else + next = toProcess[++j] or args[++i] + unless next? + throw new Error "value required for '#{argFlag}': + was the last argument provided" + switch isList + when no then next + else (options[name] or []).concat next + options # Return the help text for this **OptionParser**, listing and describing all # of the valid options, for `--help` and such. @@ -61,23 +77,37 @@ exports.OptionParser = class OptionParser # Helpers # ------- -# Regex matchers for option flags. +# Regex matchers for option flags on the command line and their rules. LONG_FLAG = /^(--\w[\w\-]*)/ SHORT_FLAG = /^(-\w)$/ MULTI_FLAG = /^-(\w{2,})/ +# Matches the long flag part of a rule for an option with an argument. Not +# applied to anything in process.argv. OPTIONAL = /\[(\w+(\*?))\]/ # Build and return the list of option rules. If the optional *short-flag* is # unspecified, leave it out by padding with `null`. -buildRules = (rules) -> - for tuple in rules +buildRules = (ruleDecls) -> + ruleList = for tuple in ruleDecls tuple.unshift null if tuple.length < 3 buildRule tuple... + flagDict = {} + for rule in ruleList + # shortFlag is null if not provided in the rule. + for flag in [rule.shortFlag, rule.longFlag] when flag? + prevRule = flagDict[flag] + if prevRule? + throw new Error "flag #{flag} for switch #{rule.name} + was already declared for switch #{prevRule.name}" + flagDict[flag] = rule + + {ruleList, flagDict} # Build a rule from a `-o` short flag, a `--output [DIR]` long flag, and the # description of what the option does. -buildRule = (shortFlag, longFlag, description, options = {}) -> +buildRule = (shortFlag, longFlag, description) -> match = longFlag.match(OPTIONAL) + shortFlag = shortFlag?.match(SHORT_FLAG)[1] longFlag = longFlag.match(LONG_FLAG)[1] { name: longFlag.substr 2 @@ -88,22 +118,11 @@ buildRule = (shortFlag, longFlag, description, options = {}) -> isList: !!(match and match[2]) } -addArgument = (rule, options, value) -> - options[rule.name] = if rule.isList - (options[rule.name] ? []).concat value - else value - -tryMatchOptionalArgument = (arg, state, rules) -> - for rule in rules - if arg in [rule.shortFlag, rule.longFlag] - if rule.hasArgument - value = state.argsLeft.shift() - if not value? - throw new Error "#{arg} requires a value, but was the last argument" - else - value = true - - addArgument(rule, state.options, value) - return +trySingleFlag = (arg) -> + if ([LONG_FLAG, SHORT_FLAG].some (pat) -> arg.match(pat)?) then [arg] + else null - throw new Error "unrecognized option: #{arg}" +trySplitMultiFlag = (arg) -> + arg.match(MULTI_FLAG)?[1] + .split('') + .map (flagName) -> "-#{flagName}" From fa3fe8b98fd7c663db3a0a6c74705d8c6c35baf3 Mon Sep 17 00:00:00 2001 From: Danny McClanahan Date: Wed, 24 May 2017 02:18:11 -0500 Subject: [PATCH 04/36] intermediate save --- src/helpers.coffee | 10 ++++ src/optparse.coffee | 6 +++ test/exec-script.coffee | 73 +++++++++++++++++++++++++++ test/exec-script/print-argv.coffee | 3 ++ test/exec-script/print-argv.litcoffee | 3 ++ test/importing.coffee | 2 +- test/support/file-helpers.coffee | 40 +++++++++++++++ test/support/helpers.coffee | 2 + 8 files changed, 138 insertions(+), 1 deletion(-) create mode 100644 test/exec-script.coffee create mode 100644 test/exec-script/print-argv.coffee create mode 100644 test/exec-script/print-argv.litcoffee create mode 100644 test/support/file-helpers.coffee diff --git a/src/helpers.coffee b/src/helpers.coffee index 617cb8ea96..a883e6c4a8 100644 --- a/src/helpers.coffee +++ b/src/helpers.coffee @@ -49,6 +49,7 @@ extend = exports.extend = (object, properties) -> exports.flatten = flatten = (array) -> flattened = [] for element in array + # TODO: is this different than Array::isArray? if not, use that if '[object Array]' is Object::toString.call element flattened = flattened.concat flatten element else @@ -67,6 +68,15 @@ exports.some = Array::some ? (fn) -> return true for e in this when fn e false +allOrderedSeqs = exports.allOrderedSeqs = (arrays...) -> + switch arrays.length + when 0 then [] + else + [cur, rest...] = arrays + recur = allOrderedSeqs rest... + nested = recur.map((seq) -> e.concat seq) for e in cur + flatten nested + # Simple function for inverting Literate CoffeeScript code by putting the # documentation in comments, producing a string of CoffeeScript code that # can be compiled "normally". diff --git a/src/optparse.coffee b/src/optparse.coffee index 1d115c5df7..0b04225e12 100644 --- a/src/optparse.coffee +++ b/src/optparse.coffee @@ -25,10 +25,14 @@ exports.OptionParser = class OptionParser # parsers that allow you to attach callback actions for every flag. Instead, # you're responsible for interpreting the options object. parse: (args) -> + console.error "args: #{args}" + options = arguments: [] skippingArgument = no originalArgs = args args = normalizeArguments args + + console.error "normalized: #{args}" for arg, i in args if skippingArgument skippingArgument = no @@ -55,6 +59,8 @@ exports.OptionParser = class OptionParser throw new Error "unrecognized option: #{arg}" if isOption and not matchedRule if seenNonOptionArg or not isOption options.arguments.push arg + + console.error "options: #{JSON.stringify(options)}" options # Return the help text for this **OptionParser**, listing and describing all diff --git a/test/exec-script.coffee b/test/exec-script.coffee new file mode 100644 index 0000000000..108acfa6fe --- /dev/null +++ b/test/exec-script.coffee @@ -0,0 +1,73 @@ +# return if global.testingBrowser +# nonBrowserTest -> + +child_process = require 'child_process' + +helpers = CoffeeScript.helpers + +binary = require.resolve '../bin/coffee' + +# shebangArgvs = [ +# [binary] +# [binary, '--'] +# ] + +scriptArgvPrefixChoices = [ + # none + [] + # one valid argument + ['-b'] + # two valid arguments + ['-b', '-j'] + # first invalid argument + ['-x', '-j'] + # second invalid argument + ['-b', '-x'] + # both invalid arguments + ['-x', '-q'] + # argument with value + ['-e', 'console.log process.argv'] +] + +scriptArgvJoinedChoices = [ + # none + [] + # two valid arguments + ['-cs'] + # first invalid argument + ['-zc'] + # second invalid argument + ['-cz'] + # two invalid arguments + ['-yz'] +] + +scriptArgvDashChoices = [ + [] + ['--'] +] + +scriptArgvFileBasename = './exec-script/print-argv' +scriptArgvPathChoices = [ + [] + require.resolve "#{scriptArgvFileBasename}.coffee" + require.resolve "#{scriptArgvFileBasename}.litcoffee" +] + +scriptArgvs = helpers.allOrderedSeqs( + # scriptArgvPathChoices, + scriptArgvPrefixChoices, + scriptArgvJoinedChoices, + scriptArgvDashChoices, + scriptArgvPathChoices) + +p = (desc, obj) -> console.error "#{desc}: #{JSON.stringify obj}" + +test "lol", -> + for argList in scriptArgvs + p 'argList', argList + output = child_process.execFileSync binary, ['--', argList...] + p 'output', output + +# test "does not modify args with '--' argument to coffee exe", -> +# for args in scriptArgvs diff --git a/test/exec-script/print-argv.coffee b/test/exec-script/print-argv.coffee new file mode 100644 index 0000000000..bb1728c208 --- /dev/null +++ b/test/exec-script/print-argv.coffee @@ -0,0 +1,3 @@ +# shebang line (or no shebang line) is added above + +console.log process.argv diff --git a/test/exec-script/print-argv.litcoffee b/test/exec-script/print-argv.litcoffee new file mode 100644 index 0000000000..bb1728c208 --- /dev/null +++ b/test/exec-script/print-argv.litcoffee @@ -0,0 +1,3 @@ +# shebang line (or no shebang line) is added above + +console.log process.argv diff --git a/test/importing.coffee b/test/importing.coffee index 96fff02028..ae301f3f1c 100644 --- a/test/importing.coffee +++ b/test/importing.coffee @@ -1,7 +1,7 @@ # Importing # --------- -unless window? or testingBrowser? +nonBrowserTest -> test "coffeescript modules can be imported and executed", -> magicKey = __filename diff --git a/test/support/file-helpers.coffee b/test/support/file-helpers.coffee new file mode 100644 index 0000000000..4ed497f7d8 --- /dev/null +++ b/test/support/file-helpers.coffee @@ -0,0 +1,40 @@ +# NOTE: unused!!! +# throw new Error 'cannot make temporary file in browser' if inBrowser() + +# fs = require 'fs' +# os = require 'os' +# path = require 'path' + +# makeTmpDir = -> +# tmpBase = path.normalize(os.tmpdir() + path.sep) +# fs.mkdtempSync tmpBase + +# # lazy single unique output directory over test run +# tmpDir = null + +# # ensures files made with makeTmpFile are unique +# counter = 0 + +# makeTmpFile = (opts = {}) -> +# { +# description = 'gen', +# suffix = '.coffee', +# contents = '', +# encoding, +# executable = no +# } = opts + +# prefix = (counter++).toString() +# fname = "#{prefix}-#{description}#{suffix}" + +# tmpDir ?= makeTmpDir() +# fpath = path.join tmpDir, fname +# toWrite = contents ? '' + +# fs.writeFileSync fpath, toWrite, {encoding, flag: 'wx+'} +# fs.chmodSync fpath, 0o0700 if executable +# fpath + +# coffeeExeFile = -> require.resolve '../../bin/coffee' + +# exports = {makeTmpFile, coffeeExeFile} diff --git a/test/support/helpers.coffee b/test/support/helpers.coffee index 3fc46508af..0c13bfea36 100644 --- a/test/support/helpers.coffee +++ b/test/support/helpers.coffee @@ -15,3 +15,5 @@ arrayEgal = (a, b) -> exports.eq = (a, b, msg) -> ok egal(a, b), msg or "Expected #{a} to equal #{b}" exports.arrayEq = (a, b, msg) -> ok arrayEgal(a,b), msg or "Expected #{a} to deep equal #{b}" +exports.inBrowser = -> window? or testingBrowser? +exports.nonBrowserTest = (scriptFunction) -> scriptFunction() unless exports.inBrowser() From aee066fba3f0d57e011f7cd89177c7469001a9d1 Mon Sep 17 00:00:00 2001 From: Danny McClanahan Date: Fri, 28 Apr 2017 02:38:15 -0500 Subject: [PATCH 05/36] add note saying where OptionParser is used in coffee command --- src/optparse.coffee | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/optparse.coffee b/src/optparse.coffee index 0b04225e12..1b1593dbbe 100644 --- a/src/optparse.coffee +++ b/src/optparse.coffee @@ -8,6 +8,9 @@ # # The first non-option is considered to be the start of the file (and file # option) list, and all subsequent arguments are left unparsed. +# +# The `coffee` command uses an instance of **OptionParser** to parse its +# command-line arguments in `src/command.coffee`. exports.OptionParser = class OptionParser # Initialize with a list of valid options, in the form: From 3c1fb7f2d71677110f66db0c695dbbebe4078e40 Mon Sep 17 00:00:00 2001 From: Danny McClanahan Date: Wed, 24 May 2017 06:05:09 -0500 Subject: [PATCH 06/36] add some more work --- src/helpers.coffee | 53 ++++++++++++++++++++++++++++++---------------- 1 file changed, 35 insertions(+), 18 deletions(-) diff --git a/src/helpers.coffee b/src/helpers.coffee index a883e6c4a8..964b1f9e05 100644 --- a/src/helpers.coffee +++ b/src/helpers.coffee @@ -28,6 +28,7 @@ exports.compact = (array) -> # Count the number of occurrences of a string in a string. exports.count = (string, substr) -> num = pos = 0 + # TODO: change 1/0 -> Infinity! return 1/0 unless substr.length num++ while pos = 1 + string.indexOf substr, pos num @@ -44,17 +45,42 @@ extend = exports.extend = (object, properties) -> object[key] = val object +# Return obj if it is an array; otherwise return an array containing only obj. +exports.toArray = toArray = (obj) -> + if Array.isArray obj then obj + else [obj] + +# Perform single flattening of an array. +# If fn provided +exports.flatten1 = flatten1 = (array, fn) -> + if not Array.isArray array then array + else array.reduce (acc, cur) -> + arg = fn?(cur) ? cur + acc.concat arg + # Return a flattened version of an array. # Handy for getting a list of `children` from the nodes. -exports.flatten = flatten = (array) -> - flattened = [] - for element in array - # TODO: is this different than Array::isArray? if not, use that - if '[object Array]' is Object::toString.call element - flattened = flattened.concat flatten element - else - flattened.push element - flattened +exports.flatten = flatten = (array) -> flatten1 array, flatten + +# Return 2D array enumerating all sequences consisting of one argument from each +# array in args. +# args consists of 2D arrays -- a non-array will be converted with toArray, as +# well as any non-array elements of the converted args. +# E.g.: ([[1, 2], [3], [[4]]], 'a') => [[1, 2, 'a'], [3, 'a'], [[4], 'a']] +exports.allOrderedSeqs = allOrderedSeqs = (args...) -> + return [] if args.length is 0 + # args may have non-array elements + [first, rest...] = args.map(toArray).reverse() + # seqs is a 2d array + seqs = first.map(toArray) + for arr in rest + # arr may have non-array elements + # each element of arr.map is an array of arrays of new permutations + seqs = flatten1 arr.map(toArray).map (subArr) -> + # construct array of arrays with subArr as prefix, seqs as suffixes + # seqs is always an array of arrays + seqs.map (pe) -> [subArr..., pe...] + seqs # Delete a key from an object, returning the value. Useful when a node is # looking for a particular method in an options hash. @@ -68,15 +94,6 @@ exports.some = Array::some ? (fn) -> return true for e in this when fn e false -allOrderedSeqs = exports.allOrderedSeqs = (arrays...) -> - switch arrays.length - when 0 then [] - else - [cur, rest...] = arrays - recur = allOrderedSeqs rest... - nested = recur.map((seq) -> e.concat seq) for e in cur - flatten nested - # Simple function for inverting Literate CoffeeScript code by putting the # documentation in comments, producing a string of CoffeeScript code that # can be compiled "normally". From a10c653a5c05e7b829f0063263cf15980e877b42 Mon Sep 17 00:00:00 2001 From: Danny McClanahan Date: Wed, 24 May 2017 07:02:17 -0500 Subject: [PATCH 07/36] fix flatten functions --- src/helpers.coffee | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/helpers.coffee b/src/helpers.coffee index 964b1f9e05..3c1055f3f2 100644 --- a/src/helpers.coffee +++ b/src/helpers.coffee @@ -40,7 +40,7 @@ exports.merge = (options, overrides) -> extend (extend {}, options), overrides # Extend a source object with the properties of another object (shallow copy). -extend = exports.extend = (object, properties) -> +exports.extend = extend = (object, properties) -> for key, val of properties object[key] = val object @@ -51,12 +51,15 @@ exports.toArray = toArray = (obj) -> else [obj] # Perform single flattening of an array. -# If fn provided +# If fn is provided, it is called on each element of array and must return the +# element to be inserted in the output. exports.flatten1 = flatten1 = (array, fn) -> - if not Array.isArray array then array - else array.reduce (acc, cur) -> + return array if not Array.isArray array + res = [] + for cur in array arg = fn?(cur) ? cur - acc.concat arg + res = res.concat arg + res # Return a flattened version of an array. # Handy for getting a list of `children` from the nodes. From c929ed9bc30cffdb11c2161d1061933120928c72 Mon Sep 17 00:00:00 2001 From: Danny McClanahan Date: Wed, 31 May 2017 11:33:07 -0500 Subject: [PATCH 08/36] refactor tests --- test/argument-parsing.coffee | 7 +++ test/exec-script.coffee | 73 --------------------------- test/exec-script/print-argv.coffee | 3 -- test/exec-script/print-argv.litcoffee | 3 -- 4 files changed, 7 insertions(+), 79 deletions(-) create mode 100644 test/argument-parsing.coffee delete mode 100644 test/exec-script.coffee delete mode 100644 test/exec-script/print-argv.coffee delete mode 100644 test/exec-script/print-argv.litcoffee diff --git a/test/argument-parsing.coffee b/test/argument-parsing.coffee new file mode 100644 index 0000000000..62997f4c40 --- /dev/null +++ b/test/argument-parsing.coffee @@ -0,0 +1,7 @@ +p = (desc, obj) -> console.error "#{desc}: #{JSON.stringify obj}" + +test "options are split after initial file name", -> + for argList in scriptArgvs + p 'argList', argList + output = child_process.execFileSync binary, ['--', argList...] + p 'output', output diff --git a/test/exec-script.coffee b/test/exec-script.coffee deleted file mode 100644 index 108acfa6fe..0000000000 --- a/test/exec-script.coffee +++ /dev/null @@ -1,73 +0,0 @@ -# return if global.testingBrowser -# nonBrowserTest -> - -child_process = require 'child_process' - -helpers = CoffeeScript.helpers - -binary = require.resolve '../bin/coffee' - -# shebangArgvs = [ -# [binary] -# [binary, '--'] -# ] - -scriptArgvPrefixChoices = [ - # none - [] - # one valid argument - ['-b'] - # two valid arguments - ['-b', '-j'] - # first invalid argument - ['-x', '-j'] - # second invalid argument - ['-b', '-x'] - # both invalid arguments - ['-x', '-q'] - # argument with value - ['-e', 'console.log process.argv'] -] - -scriptArgvJoinedChoices = [ - # none - [] - # two valid arguments - ['-cs'] - # first invalid argument - ['-zc'] - # second invalid argument - ['-cz'] - # two invalid arguments - ['-yz'] -] - -scriptArgvDashChoices = [ - [] - ['--'] -] - -scriptArgvFileBasename = './exec-script/print-argv' -scriptArgvPathChoices = [ - [] - require.resolve "#{scriptArgvFileBasename}.coffee" - require.resolve "#{scriptArgvFileBasename}.litcoffee" -] - -scriptArgvs = helpers.allOrderedSeqs( - # scriptArgvPathChoices, - scriptArgvPrefixChoices, - scriptArgvJoinedChoices, - scriptArgvDashChoices, - scriptArgvPathChoices) - -p = (desc, obj) -> console.error "#{desc}: #{JSON.stringify obj}" - -test "lol", -> - for argList in scriptArgvs - p 'argList', argList - output = child_process.execFileSync binary, ['--', argList...] - p 'output', output - -# test "does not modify args with '--' argument to coffee exe", -> -# for args in scriptArgvs diff --git a/test/exec-script/print-argv.coffee b/test/exec-script/print-argv.coffee deleted file mode 100644 index bb1728c208..0000000000 --- a/test/exec-script/print-argv.coffee +++ /dev/null @@ -1,3 +0,0 @@ -# shebang line (or no shebang line) is added above - -console.log process.argv diff --git a/test/exec-script/print-argv.litcoffee b/test/exec-script/print-argv.litcoffee deleted file mode 100644 index bb1728c208..0000000000 --- a/test/exec-script/print-argv.litcoffee +++ /dev/null @@ -1,3 +0,0 @@ -# shebang line (or no shebang line) is added above - -console.log process.argv From b1acc4bba570441be16a0ef369e22e4a85342ddf Mon Sep 17 00:00:00 2001 From: Danny McClanahan Date: Wed, 31 May 2017 12:03:06 -0500 Subject: [PATCH 09/36] make argument processing less confusing --- src/command.coffee | 4 ++-- src/optparse.coffee | 24 +++++++++++++----------- 2 files changed, 15 insertions(+), 13 deletions(-) diff --git a/src/command.coffee b/src/command.coffee index 4b3aeea29c..c63a9e4741 100644 --- a/src/command.coffee +++ b/src/command.coffee @@ -64,6 +64,7 @@ optionParser = null # Many flags cause us to divert before compiling anything. Flags passed after # `--` will be passed verbatim to your script as arguments in `process.argv` exports.run = -> + optionParser = new optparse.OptionParser SWITCHES, BANNER parseOptions() # Make the REPL *CLI* use the global context so as to (a) be consistent with the # `node` REPL CLI and, therefore, (b) make packages that modify native prototypes @@ -399,7 +400,6 @@ printTokens = (tokens) -> # Use the [OptionParser module](optparse.html) to extract all options from # `process.argv` that are specified in `SWITCHES`. parseOptions = -> - optionParser = new optparse.OptionParser SWITCHES, BANNER o = opts = optionParser.parse process.argv[2..] o.compile or= !!o.output o.run = not (o.compile or o.print or o.map) @@ -447,7 +447,7 @@ forkNode = -> # Print the `--help` usage message and exit. Deprecated switches are not # shown. usage = -> - printLine (new optparse.OptionParser SWITCHES, BANNER).help() + printLine optionParser.help() # Print the `--version` message and exit. version = -> diff --git a/src/optparse.coffee b/src/optparse.coffee index d008d5df23..42a6f7cba8 100644 --- a/src/optparse.coffee +++ b/src/optparse.coffee @@ -45,21 +45,23 @@ exports.OptionParser = class OptionParser for argFlag, j in toProcess rule = @rules.flagDict[argFlag] unless rule? - context = if multi? then " (in multi-flag '#{cmdLineArg}')" else '' - throw new Error "unrecognized option: #{argFlag}#{context}" + msg = "unrecognized option: #{argFlag}" + msg += " (in multi-flag '#{cmdLineArg}')" if multi? + throw new Error msg {hasArgument, isList, name} = rule - options[name] = switch hasArgument - when no then true + unless hasArgument + options[name] = true + else + nextArg = toProcess[++j] ? args[++i] + unless nextArg? then throw new Error "value required for '#{argFlag}', + which was the last argument provided" + if isList + options[name] ?= [] + options[name].push nextArg else - next = toProcess[++j] or args[++i] - unless next? - throw new Error "value required for '#{argFlag}': - was the last argument provided" - switch isList - when no then next - else (options[name] or []).concat next + options[name] = nextArg options # Return the help text for this **OptionParser**, listing and describing all From f3ea781e3a37836664753dd53753fc3a152531e5 Mon Sep 17 00:00:00 2001 From: Danny McClanahan Date: Wed, 31 May 2017 12:21:30 -0500 Subject: [PATCH 10/36] add basic test --- src/command.coffee | 9 +++++++-- test/argument-parsing.coffee | 11 ++++++----- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/src/command.coffee b/src/command.coffee index 6bcf2ed4be..1984683c88 100644 --- a/src/command.coffee +++ b/src/command.coffee @@ -61,10 +61,16 @@ notSources = {} watchedDirs = {} optionParser = null +exports.buildCSOptionParser = buildCSOptionParser = -> + new optparse.OptionParser SWITCHES, BANNER + # Run `coffee` by parsing passed options and determining what action to take. # Many flags cause us to divert before compiling anything. Flags passed after # `--` will be passed verbatim to your script as arguments in `process.argv` exports.run = -> + console.error "argv:" + console.error process.argv + optionParser = buildCSOptionParser() parseOptions() # Make the REPL *CLI* use the global context so as to (a) be consistent with the # `node` REPL CLI and, therefore, (b) make packages that modify native prototypes @@ -400,7 +406,6 @@ printTokens = (tokens) -> # Use the [OptionParser module](optparse.html) to extract all options from # `process.argv` that are specified in `SWITCHES`. parseOptions = -> - optionParser = new optparse.OptionParser SWITCHES, BANNER o = opts = optionParser.parse process.argv[2..] o.compile or= !!o.output o.run = not (o.compile or o.print or o.map) @@ -449,7 +454,7 @@ forkNode = -> # Print the `--help` usage message and exit. Deprecated switches are not # shown. usage = -> - printLine (new optparse.OptionParser SWITCHES, BANNER).help() + printLine optionParser.help() # Print the `--version` message and exit. version = -> diff --git a/test/argument-parsing.coffee b/test/argument-parsing.coffee index 62997f4c40..0c2652415f 100644 --- a/test/argument-parsing.coffee +++ b/test/argument-parsing.coffee @@ -1,7 +1,8 @@ p = (desc, obj) -> console.error "#{desc}: #{JSON.stringify obj}" -test "options are split after initial file name", -> - for argList in scriptArgvs - p 'argList', argList - output = child_process.execFileSync binary, ['--', argList...] - p 'output', output +{buildCSOptionParser} = require '../src/command' + +optionParser = buildCSOptionParser() + +test "combined options are still split after initial file name", -> + args = [] From 7970e44abfe382c20f5bcaba3e281165dc4a62aa Mon Sep 17 00:00:00 2001 From: Danny McClanahan Date: Wed, 31 May 2017 12:24:59 -0500 Subject: [PATCH 11/36] remove unused file --- test/support/file-helpers.coffee | 40 -------------------------------- 1 file changed, 40 deletions(-) delete mode 100644 test/support/file-helpers.coffee diff --git a/test/support/file-helpers.coffee b/test/support/file-helpers.coffee deleted file mode 100644 index 4ed497f7d8..0000000000 --- a/test/support/file-helpers.coffee +++ /dev/null @@ -1,40 +0,0 @@ -# NOTE: unused!!! -# throw new Error 'cannot make temporary file in browser' if inBrowser() - -# fs = require 'fs' -# os = require 'os' -# path = require 'path' - -# makeTmpDir = -> -# tmpBase = path.normalize(os.tmpdir() + path.sep) -# fs.mkdtempSync tmpBase - -# # lazy single unique output directory over test run -# tmpDir = null - -# # ensures files made with makeTmpFile are unique -# counter = 0 - -# makeTmpFile = (opts = {}) -> -# { -# description = 'gen', -# suffix = '.coffee', -# contents = '', -# encoding, -# executable = no -# } = opts - -# prefix = (counter++).toString() -# fname = "#{prefix}-#{description}#{suffix}" - -# tmpDir ?= makeTmpDir() -# fpath = path.join tmpDir, fname -# toWrite = contents ? '' - -# fs.writeFileSync fpath, toWrite, {encoding, flag: 'wx+'} -# fs.chmodSync fpath, 0o0700 if executable -# fpath - -# coffeeExeFile = -> require.resolve '../../bin/coffee' - -# exports = {makeTmpFile, coffeeExeFile} From bc92ff343e5cd797906cce0d29a37950cedb90ad Mon Sep 17 00:00:00 2001 From: Danny McClanahan Date: Wed, 31 May 2017 12:36:42 -0500 Subject: [PATCH 12/36] compilation now hangs --- test/argument-parsing.coffee | 6 ++++-- test/support/helpers.coffee | 2 ++ 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/test/argument-parsing.coffee b/test/argument-parsing.coffee index 0c2652415f..acfa3ea3e7 100644 --- a/test/argument-parsing.coffee +++ b/test/argument-parsing.coffee @@ -1,8 +1,10 @@ p = (desc, obj) -> console.error "#{desc}: #{JSON.stringify obj}" -{buildCSOptionParser} = require '../src/command' +# console.error (k for k of CoffeeScript) -optionParser = buildCSOptionParser() +throw new Error "?" + +optionParser = CoffeeScript.command.buildCSOptionParser() test "combined options are still split after initial file name", -> args = [] diff --git a/test/support/helpers.coffee b/test/support/helpers.coffee index a39fe09c58..9e6a6e7853 100644 --- a/test/support/helpers.coffee +++ b/test/support/helpers.coffee @@ -15,7 +15,9 @@ arrayEgal = (a, b) -> exports.eq = (a, b, msg) -> ok egal(a, b), msg or "Expected #{a} to equal #{b}" exports.arrayEq = (a, b, msg) -> ok arrayEgal(a,b), msg or "Expected #{a} to deep equal #{b}" +# FIXME: make all tests use this function exports.inBrowser = inBrowser = -> window? or testingBrowser? +# FIXME: don't use this exports.nonBrowserTest = nonBrowserTest = (scriptFunction) -> scriptFunction() unless exports.inBrowser() From 7e0d9e0fc8190598bc32efcc031c093bf0034b01 Mon Sep 17 00:00:00 2001 From: Danny McClanahan Date: Thu, 1 Jun 2017 20:32:21 -0500 Subject: [PATCH 13/36] remove unnecessary changes --- src/command.coffee | 2 -- src/helpers.coffee | 48 +++++++------------------------------ src/optparse.coffee | 6 ----- test/importing.coffee | 2 +- test/support/helpers.coffee | 5 ---- 5 files changed, 10 insertions(+), 53 deletions(-) diff --git a/src/command.coffee b/src/command.coffee index 9c7d6bd16b..ceff61b5a2 100644 --- a/src/command.coffee +++ b/src/command.coffee @@ -68,8 +68,6 @@ exports.buildCSOptionParser = buildCSOptionParser = -> # Many flags cause us to divert before compiling anything. Flags passed after # `--` will be passed verbatim to your script as arguments in `process.argv` exports.run = -> - console.error "argv:" - console.error process.argv optionParser = buildCSOptionParser() parseOptions() # Make the REPL *CLI* use the global context so as to (a) be consistent with the diff --git a/src/helpers.coffee b/src/helpers.coffee index ee33fb9b45..f92eed8af1 100644 --- a/src/helpers.coffee +++ b/src/helpers.coffee @@ -28,7 +28,6 @@ exports.compact = (array) -> # Count the number of occurrences of a string in a string. exports.count = (string, substr) -> num = pos = 0 - # TODO: change 1/0 -> Infinity! return 1/0 unless substr.length num++ while pos = 1 + string.indexOf substr, pos num @@ -40,50 +39,21 @@ exports.merge = (options, overrides) -> extend (extend {}, options), overrides # Extend a source object with the properties of another object (shallow copy). -exports.extend = extend = (object, properties) -> +extend = exports.extend = (object, properties) -> for key, val of properties object[key] = val object -# Return obj if it is an array; otherwise return an array containing only obj. -exports.toArray = toArray = (obj) -> - if Array.isArray obj then obj - else [obj] - -# Perform single flattening of an array. -# If fn is provided, it is called on each element of array and must return the -# element to be inserted in the output. -exports.flatten1 = flatten1 = (array, fn) -> - return array if not Array.isArray array - res = [] - for cur in array - arg = fn?(cur) ? cur - res = res.concat arg - res - # Return a flattened version of an array. # Handy for getting a list of `children` from the nodes. -exports.flatten = flatten = (array) -> flatten1 array, flatten - -# Return 2D array enumerating all sequences consisting of one argument from each -# array in args. -# args consists of 2D arrays -- a non-array will be converted with toArray, as -# well as any non-array elements of the converted args. -# E.g.: ([[1, 2], [3], [[4]]], 'a') => [[1, 2, 'a'], [3, 'a'], [[4], 'a']] -exports.allOrderedSeqs = allOrderedSeqs = (args...) -> - return [] if args.length is 0 - # args may have non-array elements - [first, rest...] = args.map(toArray).reverse() - # seqs is a 2d array - seqs = first.map(toArray) - for arr in rest - # arr may have non-array elements - # each element of arr.map is an array of arrays of new permutations - seqs = flatten1 arr.map(toArray).map (subArr) -> - # construct array of arrays with subArr as prefix, seqs as suffixes - # seqs is always an array of arrays - seqs.map (pe) -> [subArr..., pe...] - seqs +exports.flatten = flatten = (array) -> + flattened = [] + for element in array + if '[object Array]' is Object::toString.call element + flattened = flattened.concat flatten element + else + flattened.push element + flattened # Delete a key from an object, returning the value. Useful when a node is # looking for a particular method in an options hash. diff --git a/src/optparse.coffee b/src/optparse.coffee index 1b1593dbbe..b107b2f7c4 100644 --- a/src/optparse.coffee +++ b/src/optparse.coffee @@ -28,14 +28,10 @@ exports.OptionParser = class OptionParser # parsers that allow you to attach callback actions for every flag. Instead, # you're responsible for interpreting the options object. parse: (args) -> - console.error "args: #{args}" - options = arguments: [] skippingArgument = no originalArgs = args args = normalizeArguments args - - console.error "normalized: #{args}" for arg, i in args if skippingArgument skippingArgument = no @@ -62,8 +58,6 @@ exports.OptionParser = class OptionParser throw new Error "unrecognized option: #{arg}" if isOption and not matchedRule if seenNonOptionArg or not isOption options.arguments.push arg - - console.error "options: #{JSON.stringify(options)}" options # Return the help text for this **OptionParser**, listing and describing all diff --git a/test/importing.coffee b/test/importing.coffee index ae301f3f1c..96fff02028 100644 --- a/test/importing.coffee +++ b/test/importing.coffee @@ -1,7 +1,7 @@ # Importing # --------- -nonBrowserTest -> +unless window? or testingBrowser? test "coffeescript modules can be imported and executed", -> magicKey = __filename diff --git a/test/support/helpers.coffee b/test/support/helpers.coffee index 9e6a6e7853..5ec9684156 100644 --- a/test/support/helpers.coffee +++ b/test/support/helpers.coffee @@ -15,11 +15,6 @@ arrayEgal = (a, b) -> exports.eq = (a, b, msg) -> ok egal(a, b), msg or "Expected #{a} to equal #{b}" exports.arrayEq = (a, b, msg) -> ok arrayEgal(a,b), msg or "Expected #{a} to deep equal #{b}" -# FIXME: make all tests use this function -exports.inBrowser = inBrowser = -> window? or testingBrowser? -# FIXME: don't use this -exports.nonBrowserTest = nonBrowserTest = (scriptFunction) -> - scriptFunction() unless exports.inBrowser() exports.toJS = (str) -> CoffeeScript.compile str, bare: yes From 0a05e0cca374b14cbbcd55fd52d9eb21f71ffece Mon Sep 17 00:00:00 2001 From: Danny McClanahan Date: Thu, 1 Jun 2017 20:32:28 -0500 Subject: [PATCH 14/36] add tests!!! --- test/argument-parsing.coffee | 74 +++++++++++++++++++++++++++++++++--- 1 file changed, 68 insertions(+), 6 deletions(-) diff --git a/test/argument-parsing.coffee b/test/argument-parsing.coffee index acfa3ea3e7..fa373f26f1 100644 --- a/test/argument-parsing.coffee +++ b/test/argument-parsing.coffee @@ -1,10 +1,72 @@ -p = (desc, obj) -> console.error "#{desc}: #{JSON.stringify obj}" +{buildCSOptionParser} = require '../lib/coffeescript/command' -# console.error (k for k of CoffeeScript) +optionParser = buildCSOptionParser() -throw new Error "?" - -optionParser = CoffeeScript.command.buildCSOptionParser() +sameOptions = (opts1, opts2, msg) -> + ownKeys = Object.keys(opts1).sort() + otherKeys = Object.keys(opts2).sort() + arrayEq ownKeys, otherKeys, msg + for k in ownKeys + arrayEq opts1[k], opts2[k], msg + yes test "combined options are still split after initial file name", -> - args = [] + argv = ['some-file.coffee', '-bc'] + parsed = optionParser.parse argv + expected = arguments: ['some-file.coffee', '-b', '-c'] + sameOptions parsed, expected + + argv = ['some-file.litcoffee', '-bc'] + parsed = optionParser.parse argv + expected = arguments: ['some-file.litcoffee', '-b', '-c'] + sameOptions parsed, expected + + argv = ['-c', 'some-file.coffee', '-bc'] + parsed = optionParser.parse argv + expected = + compile: yes + arguments: ['some-file.coffee', '-b', '-c'] + sameOptions parsed, expected + + argv = ['-bc', 'some-file.coffee', '-bc'] + parsed = optionParser.parse argv + expected = + bare: yes + compile: yes + arguments: ['some-file.coffee', '-b', '-c'] + sameOptions parsed, expected + +test "combined options are not split after a '--'", -> + argv = ['--', '-bc'] + parsed = optionParser.parse argv + expected = arguments: ['-bc'] + sameOptions parsed, expected + + argv = ['-bc', '--', '-bc'] + parsed = optionParser.parse argv + expected = + bare: yes + compile: yes + arguments: ['-bc'] + sameOptions parsed, expected + +test "options are not split after any '--'", -> + argv = ['--', '--', '-bc'] + parsed = optionParser.parse argv + expected = arguments: ['--', '-bc'] + sameOptions parsed, expected + + argv = ['some-file.coffee', '--', '-bc'] + parsed = optionParser.parse argv + expected = arguments: ['some-file.coffee', '-bc'] + sameOptions parsed, expected + + argv = ['--', 'some-file.coffee', '--', 'arg'] + parsed = optionParser.parse argv + expected = arguments: ['some-file.coffee', '--', 'arg'] + sameOptions parsed, expected + + argv = ['--', 'arg', 'some-file.coffee', '--', '-bc'] + parsed = optionParser.parse argv + expected = arguments: ['arg', 'some-file.coffee', '--', '-bc'] + sameOptions parsed, expected From 42434b45b891d7a561fc56223aafe14210c9ddb0 Mon Sep 17 00:00:00 2001 From: Danny McClanahan Date: Thu, 1 Jun 2017 21:21:59 -0500 Subject: [PATCH 15/36] add/fix some tests --- src/optparse.coffee | 3 +-- test/argument-parsing.coffee | 20 +++++++++++++------- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/src/optparse.coffee b/src/optparse.coffee index c357d41419..1715266fa8 100644 --- a/src/optparse.coffee +++ b/src/optparse.coffee @@ -35,8 +35,7 @@ exports.OptionParser = class OptionParser # non-option argument are treated as non-option arguments themselves. # Executable scripts do not need to have a `--` at the end of the # shebang ("#!") line, and if they do, they won't work on Linux. - multi = trySplitMultiFlag(cmdLineArg) - toProcess = multi or trySingleFlag(cmdLineArg) + toProcess = trySplitMultiFlag(cmdLineArg) ? trySingleFlag(cmdLineArg) # If the current argument could not be parsed as one or more arguments. unless toProcess? ++i if cmdLineArg is '--' diff --git a/test/argument-parsing.coffee b/test/argument-parsing.coffee index fa373f26f1..b45c9e4987 100644 --- a/test/argument-parsing.coffee +++ b/test/argument-parsing.coffee @@ -10,22 +10,22 @@ sameOptions = (opts1, opts2, msg) -> arrayEq opts1[k], opts2[k], msg yes -test "combined options are still split after initial file name", -> +test "combined options are not split after initial file name", -> argv = ['some-file.coffee', '-bc'] parsed = optionParser.parse argv - expected = arguments: ['some-file.coffee', '-b', '-c'] + expected = arguments: ['some-file.coffee', '-bc'] sameOptions parsed, expected argv = ['some-file.litcoffee', '-bc'] parsed = optionParser.parse argv - expected = arguments: ['some-file.litcoffee', '-b', '-c'] + expected = arguments: ['some-file.litcoffee', '-bc'] sameOptions parsed, expected argv = ['-c', 'some-file.coffee', '-bc'] parsed = optionParser.parse argv expected = compile: yes - arguments: ['some-file.coffee', '-b', '-c'] + arguments: ['some-file.coffee', '-bc'] sameOptions parsed, expected argv = ['-bc', 'some-file.coffee', '-bc'] @@ -33,10 +33,10 @@ test "combined options are still split after initial file name", -> expected = bare: yes compile: yes - arguments: ['some-file.coffee', '-b', '-c'] + arguments: ['some-file.coffee', '-bc'] sameOptions parsed, expected -test "combined options are not split after a '--'", -> +test "combined options are not split after a '--', which is discarded", -> argv = ['--', '-bc'] parsed = optionParser.parse argv expected = arguments: ['-bc'] @@ -58,7 +58,7 @@ test "options are not split after any '--'", -> argv = ['some-file.coffee', '--', '-bc'] parsed = optionParser.parse argv - expected = arguments: ['some-file.coffee', '-bc'] + expected = arguments: ['some-file.coffee', '--', '-bc'] sameOptions parsed, expected argv = ['--', 'some-file.coffee', '--', 'arg'] @@ -70,3 +70,9 @@ test "options are not split after any '--'", -> parsed = optionParser.parse argv expected = arguments: ['arg', 'some-file.coffee', '--', '-bc'] sameOptions parsed, expected + +test "any non-option argument stops argument parsing", -> + argv = ['arg', '-bc'] + parsed = optionParser.parse argv + expected = arguments: ['arg', '-bc'] + sameOptions parsed, expected From 15eb624d388bf5625c0bfec9beb6e1677ad37f53 Mon Sep 17 00:00:00 2001 From: Danny McClanahan Date: Thu, 1 Jun 2017 21:24:24 -0500 Subject: [PATCH 16/36] clarify a test --- test/argument-parsing.coffee | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/test/argument-parsing.coffee b/test/argument-parsing.coffee index fa373f26f1..731d87a243 100644 --- a/test/argument-parsing.coffee +++ b/test/argument-parsing.coffee @@ -56,11 +56,6 @@ test "options are not split after any '--'", -> expected = arguments: ['--', '-bc'] sameOptions parsed, expected - argv = ['some-file.coffee', '--', '-bc'] - parsed = optionParser.parse argv - expected = arguments: ['some-file.coffee', '-bc'] - sameOptions parsed, expected - argv = ['--', 'some-file.coffee', '--', 'arg'] parsed = optionParser.parse argv expected = arguments: ['some-file.coffee', '--', 'arg'] @@ -70,3 +65,9 @@ test "options are not split after any '--'", -> parsed = optionParser.parse argv expected = arguments: ['arg', 'some-file.coffee', '--', '-bc'] sameOptions parsed, expected + +test "later '--' are removed", -> + argv = ['some-file.coffee', '--', '-bc'] + parsed = optionParser.parse argv + expected = arguments: ['some-file.coffee', '-bc'] + sameOptions parsed, expected From e1bcf84113d01d3877a3209d7e58e443d73c0b6b Mon Sep 17 00:00:00 2001 From: Danny McClanahan Date: Fri, 9 Jun 2017 13:57:31 -0400 Subject: [PATCH 17/36] fix helpers --- src/helpers.coffee | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/helpers.coffee b/src/helpers.coffee index f92eed8af1..27b965ac64 100644 --- a/src/helpers.coffee +++ b/src/helpers.coffee @@ -49,12 +49,15 @@ extend = exports.extend = (object, properties) -> exports.flatten = flatten = (array) -> flattened = [] for element in array - if '[object Array]' is Object::toString.call element - flattened = flattened.concat flatten element + # TODO: is this the same as using Object::toString.call? + if Array.isArray element + flattened.push flatten(element)... else flattened.push element flattened +exports.isString = (obj) -> Object::toString.call(obj) is '[object String]' + # Delete a key from an object, returning the value. Useful when a node is # looking for a particular method in an options hash. exports.del = (obj, key) -> From 7bc659105d929db8ea6a9310bb312adf41ae8d6b Mon Sep 17 00:00:00 2001 From: Danny McClanahan Date: Fri, 9 Jun 2017 13:57:41 -0400 Subject: [PATCH 18/36] fix opt parsing --- src/optparse.coffee | 79 +++++++++++++++++++++++++++------------------ 1 file changed, 48 insertions(+), 31 deletions(-) diff --git a/src/optparse.coffee b/src/optparse.coffee index 1715266fa8..c26427fe2d 100644 --- a/src/optparse.coffee +++ b/src/optparse.coffee @@ -1,4 +1,4 @@ -{repeat} = require './helpers' +{repeat, isString} = require './helpers' # A simple **OptionParser** class to parse option flags from the command-line. # Use it like so: @@ -28,42 +28,57 @@ exports.OptionParser = class OptionParser # parsers that allow you to attach callback actions for every flag. Instead, # you're responsible for interpreting the options object. parse: (args) -> + argsLeft = args[..] + options = {} - options = arguments: [] - for cmdLineArg, i in args + while argsLeft.length > 0 # The CS option parser is a little odd; options after the first # non-option argument are treated as non-option arguments themselves. # Executable scripts do not need to have a `--` at the end of the # shebang ("#!") line, and if they do, they won't work on Linux. - toProcess = trySplitMultiFlag(cmdLineArg) ? trySingleFlag(cmdLineArg) - # If the current argument could not be parsed as one or more arguments. - unless toProcess? - ++i if cmdLineArg is '--' - options.arguments = args[i..] - break - # Normalize arguments by expanding merged flags into multiple # flags. This allows you to have `-wl` be the same as `--watch --lint`. - for argFlag, j in toProcess - rule = @rules.flagDict[argFlag] - unless rule? - msg = "unrecognized option: #{argFlag}" - msg += " (in multi-flag '#{cmdLineArg}')" if multi? - throw new Error msg + # `flags` are objects of {multi, flag}, where `multi` contains the + # original command-line argument which was split, if applicable. + # If `arg` is not a string, then we have already processed it. + arg = argsLeft.shift() + unless isString arg then argsLeft.unshift arg + else + flags = parseMultiFlag(arg) ? parseSingleFlag(arg) + if flags? + argsLeft.unshift flags... + else + # This is a positional argument. + # TODO: should we check these with `isCoffee`? + argsLeft.unshift(arg) unless arg is '--' + break - {hasArgument, isList, name} = rule + [cur, rest...] = argsLeft + {flag, multi} = cur + rule = @rules.flagDict[flag] + unless rule? + # TODO: test all of this! + msg = "unrecognized option: #{flag}" + msg += " (in multi-flag '#{multi}')" if multi? + throw new Error msg - unless hasArgument - options[name] = true + {hasArgument, isList, name} = rule + unless hasArgument then options[name] = true + else + # We do not touch flag arguments at all, but we don't know which flags + # need arguments until we get to this point, which is why we use a while + # loop above. + # TODO: test this! + nextArg = argsLeft.shift() + unless nextArg? then throw new Error "value required for + '#{flag}', which was the last argument provided" + if isList + options[name] ?= [] + options[name].push nextArg else - nextArg = toProcess[++j] ? args[++i] - unless nextArg? then throw new Error "value required for '#{argFlag}', - which was the last argument provided" - if isList - options[name] ?= [] - options[name].push nextArg - else - options[name] = nextArg + options[name] = nextArg + + options.arguments = argsLeft options # Return the help text for this **OptionParser**, listing and describing all @@ -122,11 +137,13 @@ buildRule = (shortFlag, longFlag, description) -> isList: !!(match and match[2]) } -trySingleFlag = (arg) -> - if ([LONG_FLAG, SHORT_FLAG].some (pat) -> arg.match(pat)?) then [arg] +parseSingleFlag = (arg) -> + if ([LONG_FLAG, SHORT_FLAG].some (pat) -> arg.match(pat)?) then [flag: arg] else null -trySplitMultiFlag = (arg) -> +parseMultiFlag = (arg) -> arg.match(MULTI_FLAG)?[1] .split('') - .map (flagName) -> "-#{flagName}" + .map (flagName) -> + multi: arg + flag: "-#{flagName}" From 7c4723d699c20b02831e7c8e12d4c655563bec29 Mon Sep 17 00:00:00 2001 From: Danny McClanahan Date: Fri, 9 Jun 2017 14:35:59 -0400 Subject: [PATCH 19/36] fix infinite loop --- src/optparse.coffee | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/optparse.coffee b/src/optparse.coffee index c26427fe2d..9a663fd957 100644 --- a/src/optparse.coffee +++ b/src/optparse.coffee @@ -56,7 +56,10 @@ exports.OptionParser = class OptionParser [cur, rest...] = argsLeft {flag, multi} = cur rule = @rules.flagDict[flag] - unless rule? + if rule? + # We recognize the current top argument, so process and remove it. + argsLeft = rest + else # TODO: test all of this! msg = "unrecognized option: #{flag}" msg += " (in multi-flag '#{multi}')" if multi? From d495841189bf4d9db30c6ab655b7e333bc0dffde Mon Sep 17 00:00:00 2001 From: Danny McClanahan Date: Mon, 19 Jun 2017 01:29:57 -0400 Subject: [PATCH 20/36] make rule building easier to read --- src/optparse.coffee | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/optparse.coffee b/src/optparse.coffee index 9764ea7b43..a3b4d38f37 100644 --- a/src/optparse.coffee +++ b/src/optparse.coffee @@ -117,10 +117,10 @@ buildRules = (ruleDecls) -> for rule in ruleList # shortFlag is null if not provided in the rule. for flag in [rule.shortFlag, rule.longFlag] when flag? - prevRule = flagDict[flag] - if prevRule? + if flagDict[flag]? + # TODO: test this! throw new Error "flag #{flag} for switch #{rule.name} - was already declared for switch #{prevRule.name}" + was already declared for switch #{flagDict[flag].name}" flagDict[flag] = rule {ruleList, flagDict} @@ -132,7 +132,7 @@ buildRule = (shortFlag, longFlag, description) -> shortFlag = shortFlag?.match(SHORT_FLAG)[1] longFlag = longFlag.match(LONG_FLAG)[1] { - name: longFlag.substr 2 + name: longFlag.replace /^--/, '' shortFlag: shortFlag longFlag: longFlag description: description From bafa82fa0b398078a808e126254e4bcdd5de0152 Mon Sep 17 00:00:00 2001 From: Danny McClanahan Date: Mon, 19 Jun 2017 03:52:35 -0400 Subject: [PATCH 21/36] add tests for flag overlap --- test/option_parser.coffee | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/test/option_parser.coffee b/test/option_parser.coffee index 5401ae67bb..2f4a8186d4 100644 --- a/test/option_parser.coffee +++ b/test/option_parser.coffee @@ -41,3 +41,29 @@ test "-- and interesting combinations", -> eq undefined, result.optional eq undefined, result.required arrayEq args[1..], result.arguments + +test "throw if multiple flags try to use the same short or long name", -> + throws -> new OptionParser [ + ['-r', '--required [DIR]', 'required'] + ['-r', '--long', 'switch'] + ] + + throws -> new OptionParser [ + ['-a', '--append [STR]', 'append'] + ['-b', '--append', 'append with -b short opt'] + ] + + throws -> new OptionParser [ + ['--just-long', 'desc'] + ['--just-long', 'another desc'] + ] + + throws -> new OptionParser [ + ['-j', '--just-long', 'desc'] + ['--just-long', 'another desc'] + ] + + throws -> new OptionParser [ + ['--just-long', 'desc'] + ['-j', '--just-long', 'another desc'] + ] From 9247444c0d145eed5a2c008b73f0414b5e403ddb Mon Sep 17 00:00:00 2001 From: Danny McClanahan Date: Mon, 19 Jun 2017 03:53:19 -0400 Subject: [PATCH 22/36] revamp argument parsing again and add more thorough testing --- src/optparse.coffee | 125 +++++++++++++++++++---------------- test/argument-parsing.coffee | 18 +++++ test/option_parser.coffee | 2 - 3 files changed, 85 insertions(+), 60 deletions(-) diff --git a/src/optparse.coffee b/src/optparse.coffee index a3b4d38f37..f3f7837b21 100644 --- a/src/optparse.coffee +++ b/src/optparse.coffee @@ -28,60 +28,28 @@ exports.OptionParser = class OptionParser # parsers that allow you to attach callback actions for every flag. Instead, # you're responsible for interpreting the options object. parse: (args) -> - argsLeft = args[..] + # The CS option parser is a little odd; options after the first + # non-option argument are treated as non-option arguments themselves. + # Optional arguments are normalized by expanding merged flags into multiple + # flags. This allows you to have `-wl` be the same as `--watch --lint`. + # Note that executable scripts do not need to have a `--` at the end of the + # shebang ("#!") line, and if they do, they won't work on Linux (see #3946). + {rules, positional} = normalizeArguments args, @rules.flagDict options = {} - while argsLeft.length > 0 - # The CS option parser is a little odd; options after the first - # non-option argument are treated as non-option arguments themselves. - # Executable scripts do not need to have a `--` at the end of the - # shebang ("#!") line, and if they do, they won't work on Linux. - # Normalize arguments by expanding merged flags into multiple - # flags. This allows you to have `-wl` be the same as `--watch --lint`. - # `flags` are objects of {multi, flag}, where `multi` contains the - # original command-line argument which was split, if applicable. - # If `arg` is not a string, then we have already processed it. - arg = argsLeft.shift() - unless isString arg then argsLeft.unshift arg - else - flags = parseMultiFlag(arg) ? parseSingleFlag(arg) - if flags? - argsLeft.unshift flags... - else - # This is a positional argument. - # TODO: should we check these with `isCoffee`? - argsLeft.unshift(arg) unless arg is '--' - break - - [cur, rest...] = argsLeft - {flag, multi} = cur - rule = @rules.flagDict[flag] - if rule? - # We recognize the current top argument, so process and remove it. - argsLeft = rest - else - # TODO: test all of this! - msg = "unrecognized option: #{flag}" - msg += " (in multi-flag '#{multi}')" if multi? - throw new Error msg - - {hasArgument, isList, name} = rule - unless hasArgument then options[name] = true - else - # We do not touch flag arguments at all, but we don't know which flags - # need arguments until we get to this point, which is why we use a while - # loop above. - # TODO: test this! - nextArg = argsLeft.shift() - unless nextArg? then throw new Error "value required for - '#{flag}', which was the last argument provided" + # The `argument` field is added to the rule instance non-destructively by + # `normalizeArguments`. + for {hasArgument, argument, isList, name} in rules + if hasArgument if isList options[name] ?= [] - options[name].push nextArg + options[name].push argument else - options[name] = nextArg + options[name] = argument + else + options[name] = true - options.arguments = argsLeft + options.arguments = positional options # Return the help text for this **OptionParser**, listing and describing all @@ -118,7 +86,6 @@ buildRules = (ruleDecls) -> # shortFlag is null if not provided in the rule. for flag in [rule.shortFlag, rule.longFlag] when flag? if flagDict[flag]? - # TODO: test this! throw new Error "flag #{flag} for switch #{rule.name} was already declared for switch #{flagDict[flag].name}" flagDict[flag] = rule @@ -140,13 +107,55 @@ buildRule = (shortFlag, longFlag, description) -> isList: !!(match and match[2]) } -parseSingleFlag = (arg) -> - if ([LONG_FLAG, SHORT_FLAG].some (pat) -> arg.match(pat)?) then [flag: arg] - else null +normalizeArguments = (args, flagDict) -> + rules = [] + positional = [] + needsArgOpt = null + for arg, argIndex in args + if needsArgOpt? + # FIXME: use object spread when that gets merged (in #4493) + rules.push Object.assign({}, needsArgOpt.rule, {argument: arg}) + needsArgOpt = null + continue + + multiFlags = arg.match(MULTI_FLAG)?[1] + .split('') + .map (flagName) -> "-#{flagName}" + if multiFlags? + multiOpts = multiFlags.map (flag) -> + rule = flagDict[flag] + unless rule? + throw new Error "unrecognized option #{flag} in multi-flag #{arg}" + {rule, flag} + # Only the last flag in a multi-flag may have an argument. + [innerOpts..., lastOpt] = multiOpts + for {rule, flag} in innerOpts + if rule.hasArgument + throw new Error "cannot use option #{flag} in multi-flag #{arg} except + as the last option, because it needs an argument" + rules.push rule + if lastOpt.rule.hasArgument + needsArgOpt = lastOpt + else + rules.push lastOpt.rule + else if ([LONG_FLAG, SHORT_FLAG].some (pat) -> arg.match(pat)?) + # TODO: do we need to check if `arg` matches regex if we already check if + # it's in flagDict? + singleRule = flagDict[arg] + unless singleRule? + throw new Error "unrecognized option #{arg}" + if singleRule.hasArgument + needsArgOpt = {rule: singleRule, flag: arg} + else + rules.push singleRule + else + # This is a positional argument. + # TODO: should we check these with `isCoffee`? + finalIndex = if arg is '--' then argIndex + 1 else argIndex + positional = args[finalIndex..] + break -parseMultiFlag = (arg) -> - arg.match(MULTI_FLAG)?[1] - .split('') - .map (flagName) -> - multi: arg - flag: "-#{flagName}" + if needsArgOpt? + throw new Error "value required for #{needsArgOpt.flag}, which was the last + argument provided" + {rules, positional} diff --git a/test/argument-parsing.coffee b/test/argument-parsing.coffee index 5751172059..3aa1adbfd6 100644 --- a/test/argument-parsing.coffee +++ b/test/argument-parsing.coffee @@ -77,3 +77,21 @@ test "later '--' are not removed", -> parsed = optionParser.parse argv expected = arguments: ['some-file.coffee', '--', '-bc'] sameOptions parsed, expected + +test "throw on invalid options", -> + argv = ['-k'] + throws -> optionParser.parse argv + + argv = ['-ck'] + throws (-> optionParser.parse argv), /multi-flag/ + + argv = ['-kc'] + throws (-> optionParser.parse argv), /multi-flag/ + + argv = ['-oc'] + throws (-> optionParser.parse argv), /needs an argument/ + + # Check if all flags in a multi-flag are recognized before checking if flags + # before the last need arguments. + argv = ['-ok'] + throws (-> optionParser.parse argv), /unrecognized option/ diff --git a/test/option_parser.coffee b/test/option_parser.coffee index 2f4a8186d4..5f709924f3 100644 --- a/test/option_parser.coffee +++ b/test/option_parser.coffee @@ -1,8 +1,6 @@ # Option Parser # ------------- -# TODO: refactor option parser tests - # Ensure that the OptionParser handles arguments correctly. return unless require? {OptionParser} = require './../lib/coffeescript/optparse' From b509b464e6af0b3e255d41198684127bc26498a2 Mon Sep 17 00:00:00 2001 From: Danny McClanahan Date: Mon, 19 Jun 2017 04:13:36 -0400 Subject: [PATCH 23/36] add tests, comment, clean unused method --- src/helpers.coffee | 2 -- src/optparse.coffee | 5 ++++- test/argument-parsing.coffee | 6 ++++++ 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/src/helpers.coffee b/src/helpers.coffee index 27b965ac64..88543d3004 100644 --- a/src/helpers.coffee +++ b/src/helpers.coffee @@ -56,8 +56,6 @@ exports.flatten = flatten = (array) -> flattened.push element flattened -exports.isString = (obj) -> Object::toString.call(obj) is '[object String]' - # Delete a key from an object, returning the value. Useful when a node is # looking for a particular method in an options hash. exports.del = (obj, key) -> diff --git a/src/optparse.coffee b/src/optparse.coffee index f3f7837b21..83b906e8b0 100644 --- a/src/optparse.coffee +++ b/src/optparse.coffee @@ -1,4 +1,4 @@ -{repeat, isString} = require './helpers' +{repeat} = require './helpers' # A simple **OptionParser** class to parse option flags from the command-line. # Use it like so: @@ -112,6 +112,9 @@ normalizeArguments = (args, flagDict) -> positional = [] needsArgOpt = null for arg, argIndex in args + # If the previous argument given to the script was an option that uses the + # next command-line argument as its argument, create copy of the option's + # rule with an `argument` field. if needsArgOpt? # FIXME: use object spread when that gets merged (in #4493) rules.push Object.assign({}, needsArgOpt.rule, {argument: arg}) diff --git a/test/argument-parsing.coffee b/test/argument-parsing.coffee index 3aa1adbfd6..d1a215f526 100644 --- a/test/argument-parsing.coffee +++ b/test/argument-parsing.coffee @@ -91,6 +91,12 @@ test "throw on invalid options", -> argv = ['-oc'] throws (-> optionParser.parse argv), /needs an argument/ + argv = ['-o'] + throws (-> optionParser.parse argv), /value required/ + + argv = ['-co'] + throws (-> optionParser.parse argv), /value required/ + # Check if all flags in a multi-flag are recognized before checking if flags # before the last need arguments. argv = ['-ok'] From 31bbeb22d4c7acc53b8d33a6b0009984a5ba9b69 Mon Sep 17 00:00:00 2001 From: Danny McClanahan Date: Tue, 20 Jun 2017 13:06:53 -0400 Subject: [PATCH 24/36] address review comments --- src/command.coffee | 6 +++++- src/helpers.coffee | 5 ++--- src/optparse.coffee | 12 +++++------- 3 files changed, 12 insertions(+), 11 deletions(-) diff --git a/src/command.coffee b/src/command.coffee index ceff61b5a2..987eaa1014 100644 --- a/src/command.coffee +++ b/src/command.coffee @@ -69,7 +69,11 @@ exports.buildCSOptionParser = buildCSOptionParser = -> # `--` will be passed verbatim to your script as arguments in `process.argv` exports.run = -> optionParser = buildCSOptionParser() - parseOptions() + try parseOptions() + catch err + console.error "option parsing error: #{err.message}" + process.exit 1 + # Make the REPL *CLI* use the global context so as to (a) be consistent with the # `node` REPL CLI and, therefore, (b) make packages that modify native prototypes # (such as 'colors' and 'sugar') work as expected. diff --git a/src/helpers.coffee b/src/helpers.coffee index 88543d3004..f92eed8af1 100644 --- a/src/helpers.coffee +++ b/src/helpers.coffee @@ -49,9 +49,8 @@ extend = exports.extend = (object, properties) -> exports.flatten = flatten = (array) -> flattened = [] for element in array - # TODO: is this the same as using Object::toString.call? - if Array.isArray element - flattened.push flatten(element)... + if '[object Array]' is Object::toString.call element + flattened = flattened.concat flatten element else flattened.push element flattened diff --git a/src/optparse.coffee b/src/optparse.coffee index 83b906e8b0..2c973867d6 100644 --- a/src/optparse.coffee +++ b/src/optparse.coffee @@ -33,7 +33,7 @@ exports.OptionParser = class OptionParser # Optional arguments are normalized by expanding merged flags into multiple # flags. This allows you to have `-wl` be the same as `--watch --lint`. # Note that executable scripts do not need to have a `--` at the end of the - # shebang ("#!") line, and if they do, they won't work on Linux (see #3946). + # shebang (`#!`) line, and if they do, they won't work on Linux (see #3946). {rules, positional} = normalizeArguments args, @rules.flagDict options = {} @@ -83,7 +83,7 @@ buildRules = (ruleDecls) -> buildRule tuple... flagDict = {} for rule in ruleList - # shortFlag is null if not provided in the rule. + # `shortFlag` is null if not provided in the rule. for flag in [rule.shortFlag, rule.longFlag] when flag? if flagDict[flag]? throw new Error "flag #{flag} for switch #{rule.name} @@ -116,8 +116,8 @@ normalizeArguments = (args, flagDict) -> # next command-line argument as its argument, create copy of the option's # rule with an `argument` field. if needsArgOpt? - # FIXME: use object spread when that gets merged (in #4493) - rules.push Object.assign({}, needsArgOpt.rule, {argument: arg}) + withArg = Object.assign({}, needsArgOpt.rule, {argument: arg}) + rules.push withArg needsArgOpt = null continue @@ -142,8 +142,6 @@ normalizeArguments = (args, flagDict) -> else rules.push lastOpt.rule else if ([LONG_FLAG, SHORT_FLAG].some (pat) -> arg.match(pat)?) - # TODO: do we need to check if `arg` matches regex if we already check if - # it's in flagDict? singleRule = flagDict[arg] unless singleRule? throw new Error "unrecognized option #{arg}" @@ -159,6 +157,6 @@ normalizeArguments = (args, flagDict) -> break if needsArgOpt? - throw new Error "value required for #{needsArgOpt.flag}, which was the last + throw new Error "value required for #{needsArgOpt.flag}, but it was the last argument provided" {rules, positional} From 3944d9473b3f77178eacfa9f1c8f196b4dc64d40 Mon Sep 17 00:00:00 2001 From: Danny McClanahan Date: Wed, 21 Jun 2017 12:02:17 -0400 Subject: [PATCH 25/36] add test for direct invocation of shebang scripts --- src/optparse.coffee | 7 ++++--- test/argument-parsing.coffee | 19 +++++++++++++++++++ test/importing/shebang.coffee | 3 +++ test/support/helpers.coffee | 2 ++ 4 files changed, 28 insertions(+), 3 deletions(-) create mode 100755 test/importing/shebang.coffee diff --git a/src/optparse.coffee b/src/optparse.coffee index 2c973867d6..e3f0643d1c 100644 --- a/src/optparse.coffee +++ b/src/optparse.coffee @@ -32,8 +32,9 @@ exports.OptionParser = class OptionParser # non-option argument are treated as non-option arguments themselves. # Optional arguments are normalized by expanding merged flags into multiple # flags. This allows you to have `-wl` be the same as `--watch --lint`. - # Note that executable scripts do not need to have a `--` at the end of the - # shebang (`#!`) line, and if they do, they won't work on Linux (see #3946). + # Note that executable scripts with a shebang (`#!`) line should use the + # line `#!/usr/bin/env coffee`, or `#!/absolute/path/to/coffee`, without a + # `--` argument after, because that will fail on Linux (see #3946). {rules, positional} = normalizeArguments args, @rules.flagDict options = {} @@ -116,7 +117,7 @@ normalizeArguments = (args, flagDict) -> # next command-line argument as its argument, create copy of the option's # rule with an `argument` field. if needsArgOpt? - withArg = Object.assign({}, needsArgOpt.rule, {argument: arg}) + withArg = Object.assign {}, needsArgOpt.rule, {argument: arg} rules.push withArg needsArgOpt = null continue diff --git a/test/argument-parsing.coffee b/test/argument-parsing.coffee index d1a215f526..0070b7586e 100644 --- a/test/argument-parsing.coffee +++ b/test/argument-parsing.coffee @@ -1,3 +1,6 @@ +path = require 'path' +{execFileSync} = require 'child_process' + {buildCSOptionParser} = require '../lib/coffeescript/command' optionParser = buildCSOptionParser() @@ -101,3 +104,19 @@ test "throw on invalid options", -> # before the last need arguments. argv = ['-ok'] throws (-> optionParser.parse argv), /unrecognized option/ + +test "parses arguments for shebang scripts correctly (on unix platforms)", -> + return if isWindows() + + # Get directory containing the compiled `coffee` executable and prepend it to + # the path so `#!/usr/bin/env coffee` resolves to our locally built file. + coffeeBinDir = path.dirname require.resolve('../bin/coffee') + newPath = "#{coffeeBinDir}:#{process.env.PATH}" + newEnv = Object.assign {}, process.env, {PATH: newPath} + + shebangScript = require.resolve './importing/shebang.coffee' + stdout = execFileSync shebangScript, ['-abck'], {env: newEnv} + + expectedArgs = ['coffee', shebangScript, '-abck'] + realArgs = JSON.parse stdout + arrayEq expectedArgs, realArgs diff --git a/test/importing/shebang.coffee b/test/importing/shebang.coffee new file mode 100755 index 0000000000..fbd15960a1 --- /dev/null +++ b/test/importing/shebang.coffee @@ -0,0 +1,3 @@ +#!/usr/bin/env coffee + +process.stdout.write JSON.stringify(process.argv) diff --git a/test/support/helpers.coffee b/test/support/helpers.coffee index a7cbbc1df0..b758b4c027 100644 --- a/test/support/helpers.coffee +++ b/test/support/helpers.coffee @@ -30,3 +30,5 @@ exports.eqJS = (input, expectedOutput, msg) -> #{reset}#{expectedOutput}#{red} but instead it was: #{reset}#{actualOutput}#{red}""" + +exports.isWindows = -> process.platform is 'win32' From f41463d062f729c024e435aa216ecb326457eab8 Mon Sep 17 00:00:00 2001 From: Danny McClanahan Date: Wed, 21 Jun 2017 12:55:21 -0400 Subject: [PATCH 26/36] move shebang parsing test to separate file and check for browser --- test/argument-parsing.coffee | 19 ------------------- test/invocation-argument-parsing.coffee | 20 ++++++++++++++++++++ 2 files changed, 20 insertions(+), 19 deletions(-) create mode 100644 test/invocation-argument-parsing.coffee diff --git a/test/argument-parsing.coffee b/test/argument-parsing.coffee index 0070b7586e..d1a215f526 100644 --- a/test/argument-parsing.coffee +++ b/test/argument-parsing.coffee @@ -1,6 +1,3 @@ -path = require 'path' -{execFileSync} = require 'child_process' - {buildCSOptionParser} = require '../lib/coffeescript/command' optionParser = buildCSOptionParser() @@ -104,19 +101,3 @@ test "throw on invalid options", -> # before the last need arguments. argv = ['-ok'] throws (-> optionParser.parse argv), /unrecognized option/ - -test "parses arguments for shebang scripts correctly (on unix platforms)", -> - return if isWindows() - - # Get directory containing the compiled `coffee` executable and prepend it to - # the path so `#!/usr/bin/env coffee` resolves to our locally built file. - coffeeBinDir = path.dirname require.resolve('../bin/coffee') - newPath = "#{coffeeBinDir}:#{process.env.PATH}" - newEnv = Object.assign {}, process.env, {PATH: newPath} - - shebangScript = require.resolve './importing/shebang.coffee' - stdout = execFileSync shebangScript, ['-abck'], {env: newEnv} - - expectedArgs = ['coffee', shebangScript, '-abck'] - realArgs = JSON.parse stdout - arrayEq expectedArgs, realArgs diff --git a/test/invocation-argument-parsing.coffee b/test/invocation-argument-parsing.coffee new file mode 100644 index 0000000000..a72c89831c --- /dev/null +++ b/test/invocation-argument-parsing.coffee @@ -0,0 +1,20 @@ +return unless require? + +path = require 'path' +{execFileSync} = require 'child_process' + +test "parses arguments for shebang scripts correctly (on unix platforms)", -> + return if isWindows() + + # Get directory containing the compiled `coffee` executable and prepend it to + # the path so `#!/usr/bin/env coffee` resolves to our locally built file. + coffeeBinDir = path.dirname require.resolve('../bin/coffee') + newPath = "#{coffeeBinDir}:#{process.env.PATH}" + newEnv = Object.assign {}, process.env, {PATH: newPath} + + shebangScript = require.resolve './importing/shebang.coffee' + stdout = execFileSync shebangScript, ['-abck'], {env: newEnv} + + expectedArgs = ['coffee', shebangScript, '-abck'] + realArgs = JSON.parse stdout + arrayEq expectedArgs, realArgs From 356f3ad6c7286a5fd7e8468fa41b917dd811e8ca Mon Sep 17 00:00:00 2001 From: Danny McClanahan Date: Wed, 21 Jun 2017 14:05:21 -0400 Subject: [PATCH 27/36] remove TODO --- src/optparse.coffee | 1 - 1 file changed, 1 deletion(-) diff --git a/src/optparse.coffee b/src/optparse.coffee index e3f0643d1c..45e476511c 100644 --- a/src/optparse.coffee +++ b/src/optparse.coffee @@ -152,7 +152,6 @@ normalizeArguments = (args, flagDict) -> rules.push singleRule else # This is a positional argument. - # TODO: should we check these with `isCoffee`? finalIndex = if arg is '--' then argIndex + 1 else argIndex positional = args[finalIndex..] break From 7c17e231a86af10e93cd97cabdaa84cda8237c86 Mon Sep 17 00:00:00 2001 From: Danny McClanahan Date: Wed, 21 Jun 2017 15:49:13 -0400 Subject: [PATCH 28/36] example backwards compatible warnings --- src/coffeescript.coffee | 14 ++++++++++++++ src/command.coffee | 18 ++++++++++++++++-- src/optparse.coffee | 7 +++++-- test/argument-parsing.coffee | 17 +++++++++++++---- 4 files changed, 48 insertions(+), 8 deletions(-) diff --git a/src/coffeescript.coffee b/src/coffeescript.coffee index 0a2a9a341b..2cd6369acc 100644 --- a/src/coffeescript.coffee +++ b/src/coffeescript.coffee @@ -73,6 +73,8 @@ exports.compile = compile = withPrettyErrors (code, options) -> generateSourceMap = options.sourceMap or options.inlineMap or not options.filename? filename = options.filename or '' + checkShebangLine filename, code + sources[filename] = code map = new SourceMap if generateSourceMap @@ -295,3 +297,15 @@ Error.prepareStackTrace = (err, stack) -> " at #{formatSourcePosition frame, getSourceMapping}" "#{err.toString()}\n#{frames.join '\n'}\n" + +checkShebangLine = (file, input) -> + firstLine = input.split(/$/m)[0] + if firstLine?.match(/^#!/)? + shebangTokens = firstLine.split /\s/ + if shebangTokens.length > 2 + console.error ''' + The script to be run begins with a shebang line with more than one + argument. This script will fail on platforms such as Linux which only + allow a single argument. + ''' + console.error "The shebang line was: '#{firstLine}' in file '#{file}'" diff --git a/src/command.coffee b/src/command.coffee index 987eaa1014..47889c48bb 100644 --- a/src/command.coffee +++ b/src/command.coffee @@ -25,9 +25,11 @@ hidden = (file) -> /^\.|~$/.test file # The help banner that is printed in conjunction with `-h`/`--help`. BANNER = ''' - Usage: coffee [options] path/to/script.coffee -- [args] + Usage: coffee [options] [--] path/to/script.coffee [args] - If called without options, `coffee` will run your script. + If called without options, `coffee` will run your script. Previous versions of + coffeescript required a `--` after the script to run, but this convention is + now deprecated. ''' # The list of all the valid option flags that `coffee` knows how to handle. @@ -74,6 +76,18 @@ exports.run = -> console.error "option parsing error: #{err.message}" process.exit 1 + if (not opts.doubleDashed) and (opts.arguments[1] is '--') + printWarn ''' + coffee was invoked with '--' as the second positional argument, which is + now deprecated. To pass '--' as an argument to a script to run, put an + additional '--' before the path to your script. + + '--' will be removed from the argument list. + ''' + argStr = JSON.stringify opts.arguments + printWarn "The positional arguments were: #{argStr}" + opts.arguments = [opts.arguments[0]].concat opts.arguments[2..] + # Make the REPL *CLI* use the global context so as to (a) be consistent with the # `node` REPL CLI and, therefore, (b) make packages that modify native prototypes # (such as 'colors' and 'sugar') work as expected. diff --git a/src/optparse.coffee b/src/optparse.coffee index 45e476511c..94ecf4f4fb 100644 --- a/src/optparse.coffee +++ b/src/optparse.coffee @@ -50,6 +50,10 @@ exports.OptionParser = class OptionParser else options[name] = true + if positional[0] is '--' + options.doubleDashed = yes + positional = positional[1..] + options.arguments = positional options @@ -152,8 +156,7 @@ normalizeArguments = (args, flagDict) -> rules.push singleRule else # This is a positional argument. - finalIndex = if arg is '--' then argIndex + 1 else argIndex - positional = args[finalIndex..] + positional = args[argIndex..] break if needsArgOpt? diff --git a/test/argument-parsing.coffee b/test/argument-parsing.coffee index d1a215f526..e170786e80 100644 --- a/test/argument-parsing.coffee +++ b/test/argument-parsing.coffee @@ -39,7 +39,9 @@ test "combined options are not split after initial file name", -> test "combined options are not split after a '--', which is discarded", -> argv = ['--', '-bc'] parsed = optionParser.parse argv - expected = arguments: ['-bc'] + expected = + doubleDashed: yes + arguments: ['-bc'] sameOptions parsed, expected argv = ['-bc', '--', '-bc'] @@ -47,23 +49,30 @@ test "combined options are not split after a '--', which is discarded", -> expected = bare: yes compile: yes + doubleDashed: yes arguments: ['-bc'] sameOptions parsed, expected test "options are not split after any '--'", -> argv = ['--', '--', '-bc'] parsed = optionParser.parse argv - expected = arguments: ['--', '-bc'] + expected = + doubleDashed: yes + arguments: ['--', '-bc'] sameOptions parsed, expected argv = ['--', 'some-file.coffee', '--', 'arg'] parsed = optionParser.parse argv - expected = arguments: ['some-file.coffee', '--', 'arg'] + expected = + doubleDashed: yes + arguments: ['some-file.coffee', '--', 'arg'] sameOptions parsed, expected argv = ['--', 'arg', 'some-file.coffee', '--', '-bc'] parsed = optionParser.parse argv - expected = arguments: ['arg', 'some-file.coffee', '--', '-bc'] + expected = + doubleDashed: yes + arguments: ['arg', 'some-file.coffee', '--', '-bc'] sameOptions parsed, expected test "any non-option argument stops argument parsing", -> From 5b9b786c27b584250b68825e9d7723e5cf7f765b Mon Sep 17 00:00:00 2001 From: Danny McClanahan Date: Thu, 29 Jun 2017 09:22:27 -0500 Subject: [PATCH 29/36] add correct tests for warning 1 --- test/invocation-argument-parsing.coffee | 41 +++++++++++++++++++------ 1 file changed, 32 insertions(+), 9 deletions(-) diff --git a/test/invocation-argument-parsing.coffee b/test/invocation-argument-parsing.coffee index a72c89831c..7df4500481 100644 --- a/test/invocation-argument-parsing.coffee +++ b/test/invocation-argument-parsing.coffee @@ -1,20 +1,43 @@ return unless require? path = require 'path' -{execFileSync} = require 'child_process' +{spawnSync, execFileSync} = require 'child_process' + +# Get directory containing the compiled `coffee` executable and prepend it to +# the path so `#!/usr/bin/env coffee` resolves to our locally built file. +coffeeBinDir = path.dirname require.resolve('../bin/coffee') +patchedPath = "#{coffeeBinDir}:#{process.env.PATH}" +patchedEnv = Object.assign {}, process.env, {PATH: patchedPath} +shebangScript = require.resolve './importing/shebang.coffee' test "parses arguments for shebang scripts correctly (on unix platforms)", -> return if isWindows() - # Get directory containing the compiled `coffee` executable and prepend it to - # the path so `#!/usr/bin/env coffee` resolves to our locally built file. - coffeeBinDir = path.dirname require.resolve('../bin/coffee') - newPath = "#{coffeeBinDir}:#{process.env.PATH}" - newEnv = Object.assign {}, process.env, {PATH: newPath} - - shebangScript = require.resolve './importing/shebang.coffee' - stdout = execFileSync shebangScript, ['-abck'], {env: newEnv} + stdout = execFileSync shebangScript, ['-abck'], {env: patchedEnv} expectedArgs = ['coffee', shebangScript, '-abck'] realArgs = JSON.parse stdout arrayEq expectedArgs, realArgs + +test "warns and removes -- if it is the second positional argument", -> + result = spawnSync 'coffee', [shebangScript, '--'], {env: patchedEnv} + stderr = result.stderr.toString() + arrayEq JSON.parse(result.stdout), ['coffee', shebangScript] + ok stderr.match /^coffee was invoked with '--'/m + posArgs = stderr.match(/^The positional arguments were: (.*)$/m)[1] + arrayEq JSON.parse(posArgs), [shebangScript, '--'] + + result = spawnSync 'coffee', ['-b', shebangScript, '--'], {env: patchedEnv} + stderr = result.stderr.toString() + arrayEq JSON.parse(result.stdout), ['coffee', shebangScript] + ok stderr.match /^coffee was invoked with '--'/m + posArgs = stderr.match(/^The positional arguments were: (.*)$/m)[1] + arrayEq JSON.parse(posArgs), [shebangScript, '--'] + + result = spawnSync( + 'coffee', ['-b', shebangScript, '--', 'ANOTHER ONE'], {env: patchedEnv}) + stderr = result.stderr.toString() + arrayEq JSON.parse(result.stdout), ['coffee', shebangScript, 'ANOTHER ONE'] + ok stderr.match /^coffee was invoked with '--'/m + posArgs = stderr.match(/^The positional arguments were: (.*)$/m)[1] + arrayEq JSON.parse(posArgs), [shebangScript, '--', 'ANOTHER ONE'] From 8448a773b874c74a48fc23cdc33a48b944daf431 Mon Sep 17 00:00:00 2001 From: Danny McClanahan Date: Thu, 29 Jun 2017 10:08:53 -0500 Subject: [PATCH 30/36] add tests for warnings --- src/coffeescript.coffee | 19 ++--- src/command.coffee | 7 +- test/importing/shebang-extra-args.coffee | 3 + .../shebang-initial-space-extra-args.coffee | 3 + test/importing/shebang-initial-space.coffee | 3 + test/invocation-argument-parsing.coffee | 71 ++++++++++++++++++- 6 files changed, 89 insertions(+), 17 deletions(-) create mode 100755 test/importing/shebang-extra-args.coffee create mode 100644 test/importing/shebang-initial-space-extra-args.coffee create mode 100755 test/importing/shebang-initial-space.coffee diff --git a/src/coffeescript.coffee b/src/coffeescript.coffee index 2cd6369acc..bdc2b4ef04 100644 --- a/src/coffeescript.coffee +++ b/src/coffeescript.coffee @@ -300,12 +300,13 @@ Error.prepareStackTrace = (err, stack) -> checkShebangLine = (file, input) -> firstLine = input.split(/$/m)[0] - if firstLine?.match(/^#!/)? - shebangTokens = firstLine.split /\s/ - if shebangTokens.length > 2 - console.error ''' - The script to be run begins with a shebang line with more than one - argument. This script will fail on platforms such as Linux which only - allow a single argument. - ''' - console.error "The shebang line was: '#{firstLine}' in file '#{file}'" + rest = firstLine?.match(/^#!\s*([^\s]+\s*)(.*)/) + args = rest?[2]?.split(/\s/).filter (s) -> s isnt '' + if args?.length > 1 + console.error ''' + The script to be run begins with a shebang line with more than one + argument. This script will fail on platforms such as Linux which only + allow a single argument. + ''' + console.error "The shebang line was: '#{firstLine}' in file '#{file}'" + console.error "The arguments were: #{JSON.stringify args}" diff --git a/src/command.coffee b/src/command.coffee index 47889c48bb..ea71c8062e 100644 --- a/src/command.coffee +++ b/src/command.coffee @@ -27,9 +27,7 @@ hidden = (file) -> /^\.|~$/.test file BANNER = ''' Usage: coffee [options] [--] path/to/script.coffee [args] - If called without options, `coffee` will run your script. Previous versions of - coffeescript required a `--` after the script to run, but this convention is - now deprecated. + If called without options, `coffee` will run your script. ''' # The list of all the valid option flags that `coffee` knows how to handle. @@ -84,8 +82,7 @@ exports.run = -> '--' will be removed from the argument list. ''' - argStr = JSON.stringify opts.arguments - printWarn "The positional arguments were: #{argStr}" + printWarn "The positional arguments were: #{JSON.stringify opts.arguments}" opts.arguments = [opts.arguments[0]].concat opts.arguments[2..] # Make the REPL *CLI* use the global context so as to (a) be consistent with the diff --git a/test/importing/shebang-extra-args.coffee b/test/importing/shebang-extra-args.coffee new file mode 100755 index 0000000000..414e2faefd --- /dev/null +++ b/test/importing/shebang-extra-args.coffee @@ -0,0 +1,3 @@ +#!/usr/bin/env coffee -- + +process.stdout.write JSON.stringify(process.argv) diff --git a/test/importing/shebang-initial-space-extra-args.coffee b/test/importing/shebang-initial-space-extra-args.coffee new file mode 100644 index 0000000000..b7c8885683 --- /dev/null +++ b/test/importing/shebang-initial-space-extra-args.coffee @@ -0,0 +1,3 @@ +#! /usr/bin/env coffee extra + +process.stdout.write JSON.stringify(process.argv) diff --git a/test/importing/shebang-initial-space.coffee b/test/importing/shebang-initial-space.coffee new file mode 100755 index 0000000000..3a39f078e8 --- /dev/null +++ b/test/importing/shebang-initial-space.coffee @@ -0,0 +1,3 @@ +#! /usr/bin/env coffee + +process.stdout.write JSON.stringify(process.argv) diff --git a/test/invocation-argument-parsing.coffee b/test/invocation-argument-parsing.coffee index 7df4500481..14f0aa0595 100644 --- a/test/invocation-argument-parsing.coffee +++ b/test/invocation-argument-parsing.coffee @@ -8,24 +8,33 @@ path = require 'path' coffeeBinDir = path.dirname require.resolve('../bin/coffee') patchedPath = "#{coffeeBinDir}:#{process.env.PATH}" patchedEnv = Object.assign {}, process.env, {PATH: patchedPath} + shebangScript = require.resolve './importing/shebang.coffee' +initialSpaceScript = require.resolve './importing/shebang-initial-space.coffee' +extraArgsScript = require.resolve './importing/shebang-extra-args.coffee' +initialSpaceExtraArgsScript = require.resolve './importing/shebang-initial-space-extra-args.coffee' -test "parses arguments for shebang scripts correctly (on unix platforms)", -> +test "parse arguments for shebang scripts correctly (on unix platforms)", -> return if isWindows() stdout = execFileSync shebangScript, ['-abck'], {env: patchedEnv} - expectedArgs = ['coffee', shebangScript, '-abck'] realArgs = JSON.parse stdout arrayEq expectedArgs, realArgs -test "warns and removes -- if it is the second positional argument", -> + stdout = execFileSync initialSpaceScript, ['-abck'], {env: patchedEnv} + expectedArgs = ['coffee', initialSpaceScript, '-abck'] + realArgs = JSON.parse stdout + arrayEq expectedArgs, realArgs + +test "warn and remove -- if it is the second positional argument", -> result = spawnSync 'coffee', [shebangScript, '--'], {env: patchedEnv} stderr = result.stderr.toString() arrayEq JSON.parse(result.stdout), ['coffee', shebangScript] ok stderr.match /^coffee was invoked with '--'/m posArgs = stderr.match(/^The positional arguments were: (.*)$/m)[1] arrayEq JSON.parse(posArgs), [shebangScript, '--'] + ok result.status is 0 result = spawnSync 'coffee', ['-b', shebangScript, '--'], {env: patchedEnv} stderr = result.stderr.toString() @@ -33,6 +42,7 @@ test "warns and removes -- if it is the second positional argument", -> ok stderr.match /^coffee was invoked with '--'/m posArgs = stderr.match(/^The positional arguments were: (.*)$/m)[1] arrayEq JSON.parse(posArgs), [shebangScript, '--'] + ok result.status is 0 result = spawnSync( 'coffee', ['-b', shebangScript, '--', 'ANOTHER ONE'], {env: patchedEnv}) @@ -41,3 +51,58 @@ test "warns and removes -- if it is the second positional argument", -> ok stderr.match /^coffee was invoked with '--'/m posArgs = stderr.match(/^The positional arguments were: (.*)$/m)[1] arrayEq JSON.parse(posArgs), [shebangScript, '--', 'ANOTHER ONE'] + ok result.status is 0 + + result = spawnSync( + 'coffee', ['--', initialSpaceScript, 'arg'], {env: patchedEnv}) + expectedArgs = ['coffee', initialSpaceScript, 'arg'] + realArgs = JSON.parse result.stdout + arrayEq expectedArgs, realArgs + ok result.stderr.toString() is '' + ok result.status is 0 + +test "warn about non-portable shebang lines", -> + result = spawnSync 'coffee', [extraArgsScript, 'arg'], {env: patchedEnv} + stderr = result.stderr.toString() + arrayEq JSON.parse(result.stdout), ['coffee', extraArgsScript, 'arg'] + ok stderr.match /^The script to be run begins with a shebang line with more than one/m + [_, firstLine, file] = stderr.match(/^The shebang line was: '([^']+)' in file '([^']+)'/m) + ok (firstLine is '#!/usr/bin/env coffee --') + ok (file is extraArgsScript) + args = stderr.match(/^The arguments were: (.*)$/m)[1] + arrayEq JSON.parse(args), ['coffee', '--'] + ok result.status is 0 + + result = spawnSync 'coffee', [initialSpaceScript, 'arg'], {env: patchedEnv} + stderr = result.stderr.toString() + ok stderr is '' + arrayEq JSON.parse(result.stdout), ['coffee', initialSpaceScript, 'arg'] + ok result.status is 0 + + result = spawnSync( + 'coffee', [initialSpaceExtraArgsScript, 'arg'], {env: patchedEnv}) + stderr = result.stderr.toString() + arrayEq JSON.parse(result.stdout), ['coffee', initialSpaceExtraArgsScript, 'arg'] + ok stderr.match /^The script to be run begins with a shebang line with more than one/m + [_, firstLine, file] = stderr.match(/^The shebang line was: '([^']+)' in file '([^']+)'/m) + ok (firstLine is '#! /usr/bin/env coffee extra') + ok (file is initialSpaceExtraArgsScript) + args = stderr.match(/^The arguments were: (.*)$/m)[1] + arrayEq JSON.parse(args), ['coffee', 'extra'] + ok result.status is 0 + +test "both warnings will be shown at once", -> + result = spawnSync( + 'coffee', [initialSpaceExtraArgsScript, '--', 'arg'], {env: patchedEnv}) + stderr = result.stderr.toString() + arrayEq JSON.parse(result.stdout), ['coffee', initialSpaceExtraArgsScript, 'arg'] + ok stderr.match /^The script to be run begins with a shebang line with more than one/m + [_, firstLine, file] = stderr.match(/^The shebang line was: '([^']+)' in file '([^']+)'/m) + ok (firstLine is '#! /usr/bin/env coffee extra') + ok (file is initialSpaceExtraArgsScript) + args = stderr.match(/^The arguments were: (.*)$/m)[1] + arrayEq JSON.parse(args), ['coffee', 'extra'] + ok stderr.match /^coffee was invoked with '--'/m + posArgs = stderr.match(/^The positional arguments were: (.*)$/m)[1] + arrayEq JSON.parse(posArgs), [initialSpaceExtraArgsScript, '--', 'arg'] + ok result.status is 0 From 14df734f70b5bf500ac4d09b2a11fadb31676de2 Mon Sep 17 00:00:00 2001 From: Danny McClanahan Date: Thu, 29 Jun 2017 10:34:10 -0500 Subject: [PATCH 31/36] commit output js libs and update docs --- ...nges_argument_parsing_and_shebang_lines.md | 69 +++++++ lib/coffeescript/coffeescript.js | 17 +- lib/coffeescript/command.js | 17 +- lib/coffeescript/optparse.js | 187 +++++++++++------- 4 files changed, 216 insertions(+), 74 deletions(-) create mode 100644 documentation/sections/breaking_changes_argument_parsing_and_shebang_lines.md diff --git a/documentation/sections/breaking_changes_argument_parsing_and_shebang_lines.md b/documentation/sections/breaking_changes_argument_parsing_and_shebang_lines.md new file mode 100644 index 0000000000..f5d9cd82d9 --- /dev/null +++ b/documentation/sections/breaking_changes_argument_parsing_and_shebang_lines.md @@ -0,0 +1,69 @@ +### Argument parsing and shebang lines + +#### Trailing `--` + +Previous versions of CoffeeScript required a `--` after the script to run, but this convention is now deprecated. The new standard is described in the output of `coffee -h`: + +``` bash +> coffee -h + +Usage: coffee [options] [--] path/to/script.coffee [args] + +If called without options, `coffee` will run your script. + + +``` + +If the script is run with a `--` after the script, it will show a warning, then run your script. + +``` bash +> coffee path/to/script.coffee -- +coffee was invoked with '--' as the second positional argument, which is +now deprecated. To pass '--' as an argument to a script to run, put an +additional '--' before the path to your script. + +'--' will be removed from the argument list. +The positional arguments were: ["path/to/script.coffee","--"] +... +``` + +#### Shebang scripts + +On non-Windows platforms, a `.coffee` file can be made executable by adding a shebang (`#!`) line at the top of the file and marking the file as executable. For example: + +`executable-script.coffee`: +``` coffeescript +#!/usr/bin/env coffee + +x = 2 + 2 +console.log x +``` + +``` bash +> chmod +x ./executable-script.coffee +> ./executable-script.coffee +4 +``` + +Due to a bug in the argument parsing of previous CoffeeScript versions, this used to fail when trying to pass arguments to the script. Some users on OSX worked around the problem by using `#!/usr/bin/env coffee --` at the top of the file instead. However, that won't work on Linux, which cannot parse shebang lines with more than a single argument. While these scripts will still run on OSX, CoffeeScript will now display a warning before compiling or evaluating files that begin with a too-long shebang line: + +`invalid-executable-script.coffee`: +``` coffeescript +#!/usr/bin/env coffee -- + +x = 2 + 2 +console.log x +``` + +``` bash +> chmod +x /path/to/invalid-executable-script.coffee +> /path/to/invalid-executable-script.coffee +The script to be run begins with a shebang line with more than one +argument. This script will fail on platforms such as Linux which only +allow a single argument. +The shebang line was: '#!/usr/bin/env coffee --' in file '/path/to/shebang-extra-args.coffee' +The arguments were: ["coffee","--"] +4 +``` + +Note that the script *is* still run, producing the `4` at the bottom. diff --git a/lib/coffeescript/coffeescript.js b/lib/coffeescript/coffeescript.js index 42e44602ec..ac34df92de 100644 --- a/lib/coffeescript/coffeescript.js +++ b/lib/coffeescript/coffeescript.js @@ -1,6 +1,6 @@ // Generated by CoffeeScript 2.0.0-beta2 (function() { - var Lexer, SourceMap, base64encode, compile, formatSourcePosition, getSourceMap, helpers, lexer, packageJson, parser, sourceMaps, sources, withPrettyErrors; + var Lexer, SourceMap, base64encode, checkShebangLine, compile, formatSourcePosition, getSourceMap, helpers, lexer, packageJson, parser, sourceMaps, sources, withPrettyErrors; ({Lexer} = require('./lexer')); @@ -56,6 +56,7 @@ options = extend({}, options); generateSourceMap = options.sourceMap || options.inlineMap || (options.filename == null); filename = options.filename || ''; + checkShebangLine(filename, code); sources[filename] = code; if (generateSourceMap) { map = new SourceMap; @@ -290,4 +291,18 @@ return `${err.toString()}\n${frames.join('\n')}\n`; }; + checkShebangLine = function(file, input) { + var args, firstLine, ref, rest; + firstLine = input.split(/$/m)[0]; + rest = firstLine != null ? firstLine.match(/^#!\s*([^\s]+\s*)(.*)/) : void 0; + args = rest != null ? (ref = rest[2]) != null ? ref.split(/\s/).filter(function(s) { + return s !== ''; + }) : void 0 : void 0; + if ((args != null ? args.length : void 0) > 1) { + console.error('The script to be run begins with a shebang line with more than one\nargument. This script will fail on platforms such as Linux which only\nallow a single argument.'); + console.error(`The shebang line was: '${firstLine}' in file '${file}'`); + return console.error(`The arguments were: ${JSON.stringify(args)}`); + } + }; + }).call(this); diff --git a/lib/coffeescript/command.js b/lib/coffeescript/command.js index aa378ae565..d5ec5531a6 100644 --- a/lib/coffeescript/command.js +++ b/lib/coffeescript/command.js @@ -33,7 +33,7 @@ return /^\.|~$/.test(file); }; - BANNER = 'Usage: coffee [options] path/to/script.coffee -- [args]\n\nIf called without options, `coffee` will run your script.'; + BANNER = 'Usage: coffee [options] [--] path/to/script.coffee [args]\n\nIf called without options, `coffee` will run your script.'; SWITCHES = [['-b', '--bare', 'compile without a top-level function wrapper'], ['-c', '--compile', 'compile to JavaScript and save as .js files'], ['-e', '--eval', 'pass a string from the command line as input'], ['-h', '--help', 'display this help message'], ['-i', '--interactive', 'run an interactive CoffeeScript REPL'], ['-j', '--join [FILE]', 'concatenate the source CoffeeScript before compiling'], ['-m', '--map', 'generate source map and save as .js.map files'], ['-M', '--inline-map', 'generate source map and include it directly in output'], ['-n', '--nodes', 'print out the parse tree that the parser produces'], ['--nodejs [ARGS]', 'pass options directly to the "node" binary'], ['--no-header', 'suppress the "Generated by" header'], ['-o', '--output [DIR]', 'set the output directory for compiled JavaScript'], ['-p', '--print', 'print out the compiled JavaScript'], ['-r', '--require [MODULE*]', 'require the given module before eval or REPL'], ['-s', '--stdio', 'listen for and compile scripts over stdio'], ['-l', '--literate', 'treat stdio as literate style coffeescript'], ['-t', '--tokens', 'print out the tokens that the lexer/rewriter produce'], ['-v', '--version', 'display the version number'], ['-w', '--watch', 'watch scripts for changes and rerun commands']]; @@ -54,9 +54,20 @@ }; exports.run = function() { - var i, len, literals, ref, replCliOpts, results, source; + var err, i, len, literals, ref, replCliOpts, results, source; optionParser = buildCSOptionParser(); - parseOptions(); + try { + parseOptions(); + } catch (error) { + err = error; + console.error(`option parsing error: ${err.message}`); + process.exit(1); + } + if ((!opts.doubleDashed) && (opts.arguments[1] === '--')) { + printWarn('coffee was invoked with \'--\' as the second positional argument, which is\nnow deprecated. To pass \'--\' as an argument to a script to run, put an\nadditional \'--\' before the path to your script.\n\n\'--\' will be removed from the argument list.'); + printWarn(`The positional arguments were: ${JSON.stringify(opts.arguments)}`); + opts.arguments = [opts.arguments[0]].concat(opts.arguments.slice(2)); + } replCliOpts = { useGlobal: true }; diff --git a/lib/coffeescript/optparse.js b/lib/coffeescript/optparse.js index 1c54a14433..0afa325457 100644 --- a/lib/coffeescript/optparse.js +++ b/lib/coffeescript/optparse.js @@ -1,72 +1,52 @@ // Generated by CoffeeScript 2.0.0-beta2 (function() { - var LONG_FLAG, MULTI_FLAG, OPTIONAL, OptionParser, SHORT_FLAG, buildRule, buildRules, normalizeArguments, repeat; + var LONG_FLAG, MULTI_FLAG, OPTIONAL, OptionParser, SHORT_FLAG, buildRule, buildRules, normalizeArguments, repeat, + slice = [].slice; ({repeat} = require('./helpers')); exports.OptionParser = OptionParser = class OptionParser { - constructor(rules, banner) { + constructor(ruleDecls, banner) { this.banner = banner; - this.rules = buildRules(rules); + this.rules = buildRules(ruleDecls); } parse(args) { - var arg, i, isOption, j, k, len, len1, matchedRule, options, originalArgs, pos, ref, rule, seenNonOptionArg, skippingArgument, value; - options = { - arguments: [] - }; - skippingArgument = false; - originalArgs = args; - args = normalizeArguments(args); - for (i = j = 0, len = args.length; j < len; i = ++j) { - arg = args[i]; - if (skippingArgument) { - skippingArgument = false; - continue; - } - if (arg === '--') { - pos = originalArgs.indexOf('--'); - options.arguments = options.arguments.concat(originalArgs.slice(pos + 1)); - break; - } - isOption = !!(arg.match(LONG_FLAG) || arg.match(SHORT_FLAG)); - seenNonOptionArg = options.arguments.length > 0; - if (!seenNonOptionArg) { - matchedRule = false; - ref = this.rules; - for (k = 0, len1 = ref.length; k < len1; k++) { - rule = ref[k]; - if (rule.shortFlag === arg || rule.longFlag === arg) { - value = true; - if (rule.hasArgument) { - skippingArgument = true; - value = args[i + 1]; - } - options[rule.name] = rule.isList ? (options[rule.name] || []).concat(value) : value; - matchedRule = true; - break; + var argument, hasArgument, i, isList, len, name, options, positional, rules; + ({rules, positional} = normalizeArguments(args, this.rules.flagDict)); + options = {}; + for (i = 0, len = rules.length; i < len; i++) { + ({hasArgument, argument, isList, name} = rules[i]); + if (hasArgument) { + if (isList) { + if (options[name] == null) { + options[name] = []; } + options[name].push(argument); + } else { + options[name] = argument; } - if (isOption && !matchedRule) { - throw new Error(`unrecognized option: ${arg}`); - } - } - if (seenNonOptionArg || !isOption) { - options.arguments.push(arg); + } else { + options[name] = true; } } + if (positional[0] === '--') { + options.doubleDashed = true; + positional = positional.slice(1); + } + options.arguments = positional; return options; } help() { - var j, len, letPart, lines, ref, rule, spaces; + var i, len, letPart, lines, ref, rule, spaces; lines = []; if (this.banner) { lines.unshift(`${this.banner}\n`); } ref = this.rules; - for (j = 0, len = ref.length; j < len; j++) { - rule = ref[j]; + for (i = 0, len = ref.length; i < len; i++) { + rule = ref[i]; spaces = 15 - rule.longFlag.length; spaces = spaces > 0 ? repeat(' ', spaces) : ''; letPart = rule.shortFlag ? rule.shortFlag + ', ' : ' '; @@ -85,25 +65,45 @@ OPTIONAL = /\[(\w+(\*?))\]/; - buildRules = function(rules) { - var j, len, results, tuple; - results = []; - for (j = 0, len = rules.length; j < len; j++) { - tuple = rules[j]; - if (tuple.length < 3) { - tuple.unshift(null); + buildRules = function(ruleDecls) { + var flag, flagDict, i, j, len, len1, ref, rule, ruleList, tuple; + ruleList = (function() { + var i, len, results; + results = []; + for (i = 0, len = ruleDecls.length; i < len; i++) { + tuple = ruleDecls[i]; + if (tuple.length < 3) { + tuple.unshift(null); + } + results.push(buildRule(...tuple)); + } + return results; + })(); + flagDict = {}; + for (i = 0, len = ruleList.length; i < len; i++) { + rule = ruleList[i]; + ref = [rule.shortFlag, rule.longFlag]; + for (j = 0, len1 = ref.length; j < len1; j++) { + flag = ref[j]; + if (!(flag != null)) { + continue; + } + if (flagDict[flag] != null) { + throw new Error(`flag ${flag} for switch ${rule.name} was already declared for switch ${flagDict[flag].name}`); + } + flagDict[flag] = rule; } - results.push(buildRule(...tuple)); } - return results; + return {ruleList, flagDict}; }; - buildRule = function(shortFlag, longFlag, description, options = {}) { + buildRule = function(shortFlag, longFlag, description) { var match; match = longFlag.match(OPTIONAL); + shortFlag = shortFlag != null ? shortFlag.match(SHORT_FLAG)[1] : void 0; longFlag = longFlag.match(LONG_FLAG)[1]; return { - name: longFlag.substr(2), + name: longFlag.replace(/^--/, ''), shortFlag: shortFlag, longFlag: longFlag, description: description, @@ -112,23 +112,70 @@ }; }; - normalizeArguments = function(args) { - var arg, j, k, l, len, len1, match, ref, result; - args = args.slice(0); - result = []; - for (j = 0, len = args.length; j < len; j++) { - arg = args[j]; - if (match = arg.match(MULTI_FLAG)) { - ref = match[1].split(''); - for (k = 0, len1 = ref.length; k < len1; k++) { - l = ref[k]; - result.push('-' + l); + normalizeArguments = function(args, flagDict) { + var arg, argIndex, flag, i, innerOpts, j, k, lastOpt, len, len1, multiFlags, multiOpts, needsArgOpt, positional, ref, rule, rules, singleRule, withArg; + rules = []; + positional = []; + needsArgOpt = null; + for (argIndex = i = 0, len = args.length; i < len; argIndex = ++i) { + arg = args[argIndex]; + if (needsArgOpt != null) { + withArg = Object.assign({}, needsArgOpt.rule, { + argument: arg + }); + rules.push(withArg); + needsArgOpt = null; + continue; + } + multiFlags = (ref = arg.match(MULTI_FLAG)) != null ? ref[1].split('').map(function(flagName) { + return `-${flagName}`; + }) : void 0; + if (multiFlags != null) { + multiOpts = multiFlags.map(function(flag) { + var rule; + rule = flagDict[flag]; + if (rule == null) { + throw new Error(`unrecognized option ${flag} in multi-flag ${arg}`); + } + return {rule, flag}; + }); + innerOpts = 2 <= multiOpts.length ? slice.call(multiOpts, 0, j = multiOpts.length - 1) : (j = 0, []), lastOpt = multiOpts[j++]; + for (k = 0, len1 = innerOpts.length; k < len1; k++) { + ({rule, flag} = innerOpts[k]); + if (rule.hasArgument) { + throw new Error(`cannot use option ${flag} in multi-flag ${arg} except as the last option, because it needs an argument`); + } + rules.push(rule); + } + if (lastOpt.rule.hasArgument) { + needsArgOpt = lastOpt; + } else { + rules.push(lastOpt.rule); + } + } else if ([LONG_FLAG, SHORT_FLAG].some(function(pat) { + return arg.match(pat) != null; + })) { + singleRule = flagDict[arg]; + if (singleRule == null) { + throw new Error(`unrecognized option ${arg}`); + } + if (singleRule.hasArgument) { + needsArgOpt = { + rule: singleRule, + flag: arg + }; + } else { + rules.push(singleRule); } } else { - result.push(arg); + positional = args.slice(argIndex); + break; } } - return result; + if (needsArgOpt != null) { + throw new Error(`value required for ${needsArgOpt.flag}, but it was the last argument provided`); + } + return {rules, positional}; }; }).call(this); From b9291ae54ec268880b447226bb11da84af40409c Mon Sep 17 00:00:00 2001 From: Danny McClanahan Date: Fri, 30 Jun 2017 12:19:47 -0500 Subject: [PATCH 32/36] respond to review comments also add tests for help text --- ...nges_argument_parsing_and_shebang_lines.md | 23 +++++++++- lib/coffeescript/optparse.js | 2 +- src/optparse.coffee | 2 +- ...parsing.coffee => argument_parsing.coffee} | 0 ...-args.coffee => shebang_extra_args.coffee} | 0 ...ce.coffee => shebang_initial_space.coffee} | 0 ...> shebang_initial_space_extra_args.coffee} | 0 ...fee => invocation_argument_parsing.coffee} | 6 +-- test/option_parser.coffee | 42 ++++++++++++++++++- 9 files changed, 67 insertions(+), 8 deletions(-) rename test/{argument-parsing.coffee => argument_parsing.coffee} (100%) rename test/importing/{shebang-extra-args.coffee => shebang_extra_args.coffee} (100%) rename test/importing/{shebang-initial-space.coffee => shebang_initial_space.coffee} (100%) rename test/importing/{shebang-initial-space-extra-args.coffee => shebang_initial_space_extra_args.coffee} (100%) rename test/{invocation-argument-parsing.coffee => invocation_argument_parsing.coffee} (96%) diff --git a/documentation/sections/breaking_changes_argument_parsing_and_shebang_lines.md b/documentation/sections/breaking_changes_argument_parsing_and_shebang_lines.md index f5d9cd82d9..3834d36158 100644 --- a/documentation/sections/breaking_changes_argument_parsing_and_shebang_lines.md +++ b/documentation/sections/breaking_changes_argument_parsing_and_shebang_lines.md @@ -11,6 +11,25 @@ Usage: coffee [options] [--] path/to/script.coffee [args] If called without options, `coffee` will run your script. + -b, --bare compile without a top-level function wrapper + -c, --compile compile to JavaScript and save as .js files + -e, --eval pass a string from the command line as input + -h, --help display this help message + -i, --interactive run an interactive CoffeeScript REPL + -j, --join concatenate the source CoffeeScript before compiling + -m, --map generate source map and save as .js.map files + -M, --inline-map generate source map and include it directly in output + -n, --nodes print out the parse tree that the parser produces + --nodejs pass options directly to the "node" binary + --no-header suppress the "Generated by" header + -o, --output set the output directory for compiled JavaScript + -p, --print print out the compiled JavaScript + -r, --require require the given module before eval or REPL + -s, --stdio listen for and compile scripts over stdio + -l, --literate treat stdio as literate style coffeescript + -t, --tokens print out the tokens that the lexer/rewriter produce + -v, --version display the version number + -w, --watch watch scripts for changes and rerun commands ``` @@ -45,7 +64,7 @@ console.log x 4 ``` -Due to a bug in the argument parsing of previous CoffeeScript versions, this used to fail when trying to pass arguments to the script. Some users on OSX worked around the problem by using `#!/usr/bin/env coffee --` at the top of the file instead. However, that won't work on Linux, which cannot parse shebang lines with more than a single argument. While these scripts will still run on OSX, CoffeeScript will now display a warning before compiling or evaluating files that begin with a too-long shebang line: +Due to a bug in the argument parsing of previous CoffeeScript versions, this used to fail when trying to pass arguments to the script. Some users on OS X worked around the problem by using `#!/usr/bin/env coffee --` at the top of the file instead. However, that won’t work on Linux, which cannot parse shebang lines with more than a single argument. While these scripts will still run on OSX, CoffeeScript will now display a warning before compiling or evaluating files that begin with a too-long shebang line: `invalid-executable-script.coffee`: ``` coffeescript @@ -61,7 +80,7 @@ console.log x The script to be run begins with a shebang line with more than one argument. This script will fail on platforms such as Linux which only allow a single argument. -The shebang line was: '#!/usr/bin/env coffee --' in file '/path/to/shebang-extra-args.coffee' +The shebang line was: '#!/usr/bin/env coffee --' in file '/path/to/invalid-executable-script.coffee' The arguments were: ["coffee","--"] 4 ``` diff --git a/lib/coffeescript/optparse.js b/lib/coffeescript/optparse.js index 0afa325457..cc45e951de 100644 --- a/lib/coffeescript/optparse.js +++ b/lib/coffeescript/optparse.js @@ -44,7 +44,7 @@ if (this.banner) { lines.unshift(`${this.banner}\n`); } - ref = this.rules; + ref = this.rules.ruleList; for (i = 0, len = ref.length; i < len; i++) { rule = ref[i]; spaces = 15 - rule.longFlag.length; diff --git a/src/optparse.coffee b/src/optparse.coffee index 94ecf4f4fb..3aca0cea6b 100644 --- a/src/optparse.coffee +++ b/src/optparse.coffee @@ -62,7 +62,7 @@ exports.OptionParser = class OptionParser help: -> lines = [] lines.unshift "#{@banner}\n" if @banner - for rule in @rules + for rule in @rules.ruleList spaces = 15 - rule.longFlag.length spaces = if spaces > 0 then repeat ' ', spaces else '' letPart = if rule.shortFlag then rule.shortFlag + ', ' else ' ' diff --git a/test/argument-parsing.coffee b/test/argument_parsing.coffee similarity index 100% rename from test/argument-parsing.coffee rename to test/argument_parsing.coffee diff --git a/test/importing/shebang-extra-args.coffee b/test/importing/shebang_extra_args.coffee similarity index 100% rename from test/importing/shebang-extra-args.coffee rename to test/importing/shebang_extra_args.coffee diff --git a/test/importing/shebang-initial-space.coffee b/test/importing/shebang_initial_space.coffee similarity index 100% rename from test/importing/shebang-initial-space.coffee rename to test/importing/shebang_initial_space.coffee diff --git a/test/importing/shebang-initial-space-extra-args.coffee b/test/importing/shebang_initial_space_extra_args.coffee similarity index 100% rename from test/importing/shebang-initial-space-extra-args.coffee rename to test/importing/shebang_initial_space_extra_args.coffee diff --git a/test/invocation-argument-parsing.coffee b/test/invocation_argument_parsing.coffee similarity index 96% rename from test/invocation-argument-parsing.coffee rename to test/invocation_argument_parsing.coffee index 14f0aa0595..5a5b0e6a39 100644 --- a/test/invocation-argument-parsing.coffee +++ b/test/invocation_argument_parsing.coffee @@ -10,9 +10,9 @@ patchedPath = "#{coffeeBinDir}:#{process.env.PATH}" patchedEnv = Object.assign {}, process.env, {PATH: patchedPath} shebangScript = require.resolve './importing/shebang.coffee' -initialSpaceScript = require.resolve './importing/shebang-initial-space.coffee' -extraArgsScript = require.resolve './importing/shebang-extra-args.coffee' -initialSpaceExtraArgsScript = require.resolve './importing/shebang-initial-space-extra-args.coffee' +initialSpaceScript = require.resolve './importing/shebang_initial_space.coffee' +extraArgsScript = require.resolve './importing/shebang_extra_args.coffee' +initialSpaceExtraArgsScript = require.resolve './importing/shebang_initial_space_extra_args.coffee' test "parse arguments for shebang scripts correctly (on unix platforms)", -> return if isWindows() diff --git a/test/option_parser.coffee b/test/option_parser.coffee index 5f709924f3..0897d71014 100644 --- a/test/option_parser.coffee +++ b/test/option_parser.coffee @@ -5,12 +5,18 @@ return unless require? {OptionParser} = require './../lib/coffeescript/optparse' -opt = new OptionParser [ +flags = [ ['-r', '--required [DIR]', 'desc required'] ['-o', '--optional', 'desc optional'] ['-l', '--list [FILES*]', 'desc list'] ] +banner = ''' + banner text +''' + +opt = new OptionParser flags, banner + test "basic arguments", -> args = ['one', 'two', 'three', '-r', 'dir'] result = opt.parse args @@ -65,3 +71,37 @@ test "throw if multiple flags try to use the same short or long name", -> ['--just-long', 'desc'] ['-j', '--just-long', 'another desc'] ] + +reQuote = (str) -> (str ? '').replace /[.?*+^$[\]\\(){}|-]/g, "\\$&" + +flagPattern = (flag) -> + switch flag.length + when 2 + shortFlag = null + [longFlag, desc] = flag + when 3 + [shortFlag, longFlag, desc] = flag + else + throw new Error "invalid flag" + longFlagPat = reQuote longFlag.match(/--[^\s]+/)[0] + descPat = reQuote desc + if shortFlag? + joined = [reQuote(shortFlag), longFlagPat, descPat].join '.*' + else + joined = [longFlagPat, descPat].join '.*' + new RegExp "^.*#{joined}.*$", 'm' + +test "help text shows banner and all switches", -> + bannerHelp = opt.help() + bannerPattern = new RegExp reQuote(banner), 'g' + ok bannerHelp.match bannerPattern + for flag in flags + linePat = flagPattern flag + ok bannerHelp.match linePat + +test "help text shows switches, even without banner", -> + parser = new OptionParser flags + noBannerHelp = parser.help() + for flag in flags + linePat = flagPattern flag + ok noBannerHelp.match linePat From f3068592616b366281e0aef62e47249a398f9856 Mon Sep 17 00:00:00 2001 From: Danny McClanahan Date: Fri, 7 Jul 2017 17:48:18 -0500 Subject: [PATCH 33/36] respond to review comments --- ...nges_argument_parsing_and_shebang_lines.md | 51 ++++++++---------- test/argument_parsing.coffee | 29 ++++++++++ test/option_parser.coffee | 53 +++++++------------ 3 files changed, 72 insertions(+), 61 deletions(-) diff --git a/documentation/sections/breaking_changes_argument_parsing_and_shebang_lines.md b/documentation/sections/breaking_changes_argument_parsing_and_shebang_lines.md index 3834d36158..27d76c4fd7 100644 --- a/documentation/sections/breaking_changes_argument_parsing_and_shebang_lines.md +++ b/documentation/sections/breaking_changes_argument_parsing_and_shebang_lines.md @@ -1,39 +1,27 @@ ### Argument parsing and shebang lines +Version 2 deprecates multiple ways to invoke the `coffee` command that were deemed inconsistent. Invoking `coffee` in these ways will work as before, but will print a warning to the standard error stream. + #### Trailing `--` Previous versions of CoffeeScript required a `--` after the script to run, but this convention is now deprecated. The new standard is described in the output of `coffee -h`: +*output:* ``` bash > coffee -h Usage: coffee [options] [--] path/to/script.coffee [args] +... +``` -If called without options, `coffee` will run your script. - - -b, --bare compile without a top-level function wrapper - -c, --compile compile to JavaScript and save as .js files - -e, --eval pass a string from the command line as input - -h, --help display this help message - -i, --interactive run an interactive CoffeeScript REPL - -j, --join concatenate the source CoffeeScript before compiling - -m, --map generate source map and save as .js.map files - -M, --inline-map generate source map and include it directly in output - -n, --nodes print out the parse tree that the parser produces - --nodejs pass options directly to the "node" binary - --no-header suppress the "Generated by" header - -o, --output set the output directory for compiled JavaScript - -p, --print print out the compiled JavaScript - -r, --require require the given module before eval or REPL - -s, --stdio listen for and compile scripts over stdio - -l, --literate treat stdio as literate style coffeescript - -t, --tokens print out the tokens that the lexer/rewriter produce - -v, --version display the version number - -w, --watch watch scripts for changes and rerun commands +If the script is run with a `--` after the script, it will show a warning, then run your script. +*script.coffee:* +``` coffeescript +console.log 'hello world!' ``` -If the script is run with a `--` after the script, it will show a warning, then run your script. +*output:* ``` bash > coffee path/to/script.coffee -- @@ -43,30 +31,36 @@ additional '--' before the path to your script. '--' will be removed from the argument list. The positional arguments were: ["path/to/script.coffee","--"] -... +hello world! ``` +Note that the script is still run as usual, printing `"hello world!"` after the warning text. + #### Shebang scripts +##### Correct Example + On non-Windows platforms, a `.coffee` file can be made executable by adding a shebang (`#!`) line at the top of the file and marking the file as executable. For example: -`executable-script.coffee`: +*executable-script.coffee:* ``` coffeescript #!/usr/bin/env coffee x = 2 + 2 console.log x ``` - +*output:* ``` bash > chmod +x ./executable-script.coffee > ./executable-script.coffee 4 ``` -Due to a bug in the argument parsing of previous CoffeeScript versions, this used to fail when trying to pass arguments to the script. Some users on OS X worked around the problem by using `#!/usr/bin/env coffee --` at the top of the file instead. However, that won’t work on Linux, which cannot parse shebang lines with more than a single argument. While these scripts will still run on OSX, CoffeeScript will now display a warning before compiling or evaluating files that begin with a too-long shebang line: +##### Incorrect Example + +Due to a bug in the argument parsing of previous CoffeeScript versions, this used to fail when trying to pass arguments to the script. Some users on OSX worked around the problem by using `#!/usr/bin/env coffee --` at the top of the file instead. However, that won’t work on Linux, which cannot parse shebang lines with more than a single argument. While these scripts will still run on OSX, CoffeeScript will now display a warning before compiling or evaluating files that begin with a too-long shebang line: -`invalid-executable-script.coffee`: +*invalid-executable-script.coffee:* ``` coffeescript #!/usr/bin/env coffee -- @@ -74,6 +68,7 @@ x = 2 + 2 console.log x ``` +*output:* ``` bash > chmod +x /path/to/invalid-executable-script.coffee > /path/to/invalid-executable-script.coffee @@ -85,4 +80,4 @@ The arguments were: ["coffee","--"] 4 ``` -Note that the script *is* still run, producing the `4` at the bottom. +Note that the script *is* still run as normal, producing the `4` at the bottom. diff --git a/test/argument_parsing.coffee b/test/argument_parsing.coffee index c36b0374a4..fdd01b234f 100644 --- a/test/argument_parsing.coffee +++ b/test/argument_parsing.coffee @@ -111,3 +111,32 @@ test "throw on invalid options", -> # before the last need arguments. argv = ['-ok'] throws (-> optionParser.parse argv), /unrecognized option/ + +test "has expected help text", -> + ok optionParser.help() is ''' + +Usage: coffee [options] [--] path/to/script.coffee [args] + +If called without options, `coffee` will run your script. + + -b, --bare compile without a top-level function wrapper + -c, --compile compile to JavaScript and save as .js files + -e, --eval pass a string from the command line as input + -h, --help display this help message + -i, --interactive run an interactive CoffeeScript REPL + -j, --join concatenate the source CoffeeScript before compiling + -m, --map generate source map and save as .js.map files + -M, --inline-map generate source map and include it directly in output + -n, --nodes print out the parse tree that the parser produces + --nodejs pass options directly to the "node" binary + --no-header suppress the "Generated by" header + -o, --output set the output directory for compiled JavaScript + -p, --print print out the compiled JavaScript + -r, --require require the given module before eval or REPL + -s, --stdio listen for and compile scripts over stdio + -l, --literate treat stdio as literate style coffeescript + -t, --tokens print out the tokens that the lexer/rewriter produce + -v, --version display the version number + -w, --watch watch scripts for changes and rerun commands + + ''' diff --git a/test/option_parser.coffee b/test/option_parser.coffee index 0897d71014..3449899cfe 100644 --- a/test/option_parser.coffee +++ b/test/option_parser.coffee @@ -72,36 +72,23 @@ test "throw if multiple flags try to use the same short or long name", -> ['-j', '--just-long', 'another desc'] ] -reQuote = (str) -> (str ? '').replace /[.?*+^$[\]\\(){}|-]/g, "\\$&" - -flagPattern = (flag) -> - switch flag.length - when 2 - shortFlag = null - [longFlag, desc] = flag - when 3 - [shortFlag, longFlag, desc] = flag - else - throw new Error "invalid flag" - longFlagPat = reQuote longFlag.match(/--[^\s]+/)[0] - descPat = reQuote desc - if shortFlag? - joined = [reQuote(shortFlag), longFlagPat, descPat].join '.*' - else - joined = [longFlagPat, descPat].join '.*' - new RegExp "^.*#{joined}.*$", 'm' - -test "help text shows banner and all switches", -> - bannerHelp = opt.help() - bannerPattern = new RegExp reQuote(banner), 'g' - ok bannerHelp.match bannerPattern - for flag in flags - linePat = flagPattern flag - ok bannerHelp.match linePat - -test "help text shows switches, even without banner", -> - parser = new OptionParser flags - noBannerHelp = parser.help() - for flag in flags - linePat = flagPattern flag - ok noBannerHelp.match linePat +test "outputs expected help text", -> + expectedBanner = ''' + +banner text + + -r, --required desc required + -o, --optional desc optional + -l, --list desc list + + ''' + ok opt.help() is expectedBanner + + expected = [ + '' + ' -r, --required desc required' + ' -o, --optional desc optional' + ' -l, --list desc list' + '' + ].join('\n') + ok new OptionParser(flags).help() is expected From 4856fd69b7253d1e0c987777dceb748eb06b7309 Mon Sep 17 00:00:00 2001 From: Danny McClanahan Date: Fri, 7 Jul 2017 17:53:56 -0500 Subject: [PATCH 34/36] fix example output --- .../breaking_changes_argument_parsing_and_shebang_lines.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/documentation/sections/breaking_changes_argument_parsing_and_shebang_lines.md b/documentation/sections/breaking_changes_argument_parsing_and_shebang_lines.md index 27d76c4fd7..e6eea8753e 100644 --- a/documentation/sections/breaking_changes_argument_parsing_and_shebang_lines.md +++ b/documentation/sections/breaking_changes_argument_parsing_and_shebang_lines.md @@ -34,7 +34,7 @@ The positional arguments were: ["path/to/script.coffee","--"] hello world! ``` -Note that the script is still run as usual, printing `"hello world!"` after the warning text. +Note that the script is still run as usual, printing `hello world!` after the warning text. #### Shebang scripts From f7e8c2b3941d8172304c23dce7ad9816b1f9fb84 Mon Sep 17 00:00:00 2001 From: Geoffrey Booth Date: Sat, 8 Jul 2017 19:30:04 -0700 Subject: [PATCH 35/36] Rewrite argument parsing documentation to be more concise; add it to sidebar and body; add new output --- docs/v2/index.html | 38 +++++++-- ...nges_argument_parsing_and_shebang_lines.md | 77 ++++--------------- ...he_less_than_and_greater_than_operators.md | 2 +- documentation/sections/coffeescript_2.md | 2 +- documentation/v2/body.html | 3 + documentation/v2/docs.css | 4 +- documentation/v2/sidebar.html | 3 + 7 files changed, 56 insertions(+), 73 deletions(-) diff --git a/docs/v2/index.html b/docs/v2/index.html index 5ddc172cbc..ac291e2b49 100644 --- a/docs/v2/index.html +++ b/docs/v2/index.html @@ -306,7 +306,7 @@ white-space: nowrap; } -h2, h3 { +h2, h3, h4 { margin-top: 1.3em; margin-bottom: 0.6em; font-family: 'Alegreya Sans'; @@ -314,7 +314,7 @@ h2 { font-weight: 800; } -h3, h2 time { +h3, h4, h2 time { font-weight: 400; } @@ -734,6 +734,9 @@ +