From c26903e30efd96fb12f5cc8f183e1f08ad0b1379 Mon Sep 17 00:00:00 2001 From: Natalie Weizenbaum Date: Thu, 31 May 2018 20:38:45 -0400 Subject: [PATCH] Don't consider drive separators to be path-separating colons (#342) Closes #340 --- CHANGELOG.md | 5 ++++ lib/src/executable/options.dart | 50 +++++++++++++++++++++++++-------- lib/src/logger.dart | 1 - pubspec.yaml | 2 +- test/cli/shared.dart | 18 ++++++++++++ test/cli/shared/colon_args.dart | 16 +++++++++++ 6 files changed, 78 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c0238de9..77f66fa3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,8 @@ +## 1.5.1 + +* Fix a bug where an absolute Windows path would be considered an `input:output` + pair. + ## 1.5.0 * Fix a bug where an importer would be passed an incorrectly-resolved URL when diff --git a/lib/src/executable/options.dart b/lib/src/executable/options.dart index 466232e8..3e42a643 100644 --- a/lib/src/executable/options.dart +++ b/lib/src/executable/options.dart @@ -3,10 +3,12 @@ // https://opensource.org/licenses/MIT. import 'package:args/args.dart'; +import 'package:charcode/charcode.dart'; import 'package:meta/meta.dart'; import '../../sass.dart'; import '../io.dart'; +import '../util/character.dart'; import '../util/path.dart'; /// The parsed and processed command-line options for the Sass executable. @@ -180,12 +182,18 @@ class ExecutableOptions { var colonArgs = false; var positionalArgs = false; for (var argument in _options.rest) { - if (argument.isEmpty) { - _fail('Invalid argument "".'); - } else if (argument.contains(":")) { - colonArgs = true; - } else { + if (argument.isEmpty) _fail('Invalid argument "".'); + + // If the colon appears at position 1, treat it as a Windows drive + // letter. + if (!argument.contains(":") || + (_isWindowsPath(argument, 0) && + // Look for colons after index 1, since that's where the drive + // letter is on Windows paths. + argument.indexOf(":", 2) == -1)) { positionalArgs = true; + } else { + colonArgs = true; } } @@ -220,14 +228,26 @@ class ExecutableOptions { var seen = new Set(); var sourcesToDestinations = {}; for (var argument in _options.rest) { - var components = argument.split(":"); - if (components.length > 2) { - _fail('"$argument" may only contain one ":".'); - } - assert(components.length == 2); + String source; + String destination; + for (var i = 0; i < argument.length; i++) { + // A colon at position 1 may be a Windows drive letter and not a + // separator. + if (i == 1 && _isWindowsPath(argument, i - 1)) continue; + + if (argument.codeUnitAt(i) == $colon) { + if (source == null) { + source = argument.substring(0, i); + destination = argument.substring(i + 1); + } else if (i != source.length + 2 || + !_isWindowsPath(argument, i - 1)) { + // A colon 2 character after the separator may also be a Windows + // drive letter. + _fail('"$argument" may only contain one ":".'); + } + } + } - var source = components.first; - var destination = components.last; if (!seen.add(source)) { _fail('Duplicate source "${source}".'); } @@ -246,6 +266,12 @@ class ExecutableOptions { Map _sourcesToDestinations; + /// Returns whether [string] contains an absolute Windows path at [index]. + bool _isWindowsPath(String string, int index) => + string.length > index + 2 && + isAlphabetic(string.codeUnitAt(index)) && + string.codeUnitAt(index + 1) == $colon; + /// Returns the sub-map of [sourcesToDestinations] for the given [source] and /// [destination] directories. Map _listSourceDirectory(String source, String destination) { diff --git a/lib/src/logger.dart b/lib/src/logger.dart index 01c3d680..c8d2de41 100644 --- a/lib/src/logger.dart +++ b/lib/src/logger.dart @@ -6,7 +6,6 @@ import 'package:source_span/source_span.dart'; import 'package:stack_trace/stack_trace.dart'; import 'logger/stderr.dart'; -import 'util/path.dart'; /// An interface for loggers that print messages produced by Sass stylesheets. /// diff --git a/pubspec.yaml b/pubspec.yaml index 2a6f3c41..b5c43193 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -1,5 +1,5 @@ name: sass -version: 1.5.0 +version: 1.5.1 description: A Sass implementation in Dart. author: Dart Team homepage: https://github.com/sass/dart-sass diff --git a/test/cli/shared.dart b/test/cli/shared.dart index 37e24dac..52fda601 100644 --- a/test/cli/shared.dart +++ b/test/cli/shared.dart @@ -8,6 +8,8 @@ import 'package:test/test.dart'; import 'package:test_descriptor/test_descriptor.dart' as d; import 'package:test_process/test_process.dart'; +import 'package:sass/src/util/path.dart'; + /// Defines test that are shared between the Dart and Node.js CLI test suites. void sharedTests(Future runSass(Iterable arguments)) { /// Runs the executable on [arguments] plus an output file, then verifies that @@ -43,6 +45,22 @@ void sharedTests(Future runSass(Iterable arguments)) { await sass.shouldExit(0); }); + // On Windows, this verifies that we don't consider the colon after a drive + // letter to be an `input:output` separator. + test("compiles an absolute Sass file to CSS", () async { + await d.file("test.scss", "a {b: 1 + 2}").create(); + + var sass = await runSass([p.absolute(p.join(d.sandbox, "test.scss"))]); + expect( + sass.stdout, + emitsInOrder([ + "a {", + " b: 3;", + "}", + ])); + await sass.shouldExit(0); + }); + test("writes a CSS file to disk", () async { await d.file("test.scss", "a {b: 1 + 2}").create(); diff --git a/test/cli/shared/colon_args.dart b/test/cli/shared/colon_args.dart index af9c5b7e..7c2c9a48 100644 --- a/test/cli/shared/colon_args.dart +++ b/test/cli/shared/colon_args.dart @@ -8,6 +8,8 @@ import 'package:test/test.dart'; import 'package:test_descriptor/test_descriptor.dart' as d; import 'package:test_process/test_process.dart'; +import 'package:sass/src/util/path.dart'; + /// Defines test that are shared between the Dart and Node.js CLI test suites. void sharedTests(Future runSass(Iterable arguments)) { test("compiles multiple sources to multiple destinations", () async { @@ -27,6 +29,20 @@ void sharedTests(Future runSass(Iterable arguments)) { .validate(); }); + // On Windows, this verifies that we don't consider the colon after a drive + // letter to be an `input:output` separator. + test("compiles an absolute source to an absolute destination", () async { + await d.file("test.scss", "a {b: c}").create(); + + var input = p.absolute(p.join(d.sandbox, 'test.scss')); + var output = p.absolute(p.join(d.sandbox, 'out.css')); + var sass = await runSass(["--no-source-map", "$input:$output"]); + expect(sass.stdout, emitsDone); + await sass.shouldExit(0); + + await d.file("out.css", equalsIgnoringWhitespace("a { b: c; }")).validate(); + }); + test("creates destination directories", () async { await d.file("test.scss", "a {b: c}").create();