From 144cd355154b4366dc2d74a3700fc0eeb81da7d4 Mon Sep 17 00:00:00 2001 From: Michael Crumm Date: Thu, 29 Jul 2021 12:34:44 -0700 Subject: [PATCH 1/4] Abort sass if stdin is closed when watching --- bin/sass.dart | 1 + lib/src/io/interface.dart | 8 ++++++++ lib/src/io/node.dart | 9 +++++++++ lib/src/io/vm.dart | 4 ++++ 4 files changed, 22 insertions(+) diff --git a/bin/sass.dart b/bin/sass.dart index b49cdd4e2..bc8bcebe9 100644 --- a/bin/sass.dart +++ b/bin/sass.dart @@ -54,6 +54,7 @@ Future main(List args) async { var graph = StylesheetGraph( ImportCache(loadPaths: options.loadPaths, logger: options.logger)); if (options.watch) { + ensureWatchWillExit(); await watch(options, graph); return; } diff --git a/lib/src/io/interface.dart b/lib/src/io/interface.dart index 0ee35bc53..4784b0d41 100644 --- a/lib/src/io/interface.dart +++ b/lib/src/io/interface.dart @@ -94,6 +94,14 @@ String? getEnvironmentVariable(String name) => throw ''; int get exitCode => throw ''; set exitCode(int value) => throw ''; +/// Attaches a listener to exit when stdin closes. +/// +/// The listener is *not* attached when stdin is a TTY because it would +/// interfere with the Unix background job system. If we read from stdin and +/// then Ctrl+Z to move the process to the background, it will incorrectly +/// cause the job to stop. See: https://github.com/brunch/brunch/issues/998. +void ensureWatchWillExit() => throw ''; + /// Recursively watches the directory at [path] for modifications. /// /// Returns a future that completes with a single-subscription stream once the diff --git a/lib/src/io/node.dart b/lib/src/io/node.dart index f8ac7f129..62896488e 100644 --- a/lib/src/io/node.dart +++ b/lib/src/io/node.dart @@ -196,6 +196,9 @@ final stderr = Stderr(process.stderr); @JS('process.stdout.isTTY') external bool? get isTTY; +@JS('process.stdin.isTTY') +external bool? get isStdinTTY; + bool get hasTerminal => isTTY == true; bool get isWindows => process.platform == 'win32'; @@ -213,6 +216,12 @@ int get exitCode => process.exitCode; set exitCode(int code) => process.exitCode = code; +void ensureWatchWillExit() { + if (isStdinTTY == true) { + process.stdin.on('end', allowInterop(() => process.exit(0))); + } +} + Future> watchDir(String path, {bool poll = false}) { var watcher = chokidar.watch( path, ChokidarOptions(disableGlobbing: true, usePolling: poll)); diff --git a/lib/src/io/vm.dart b/lib/src/io/vm.dart index d6ab96c1c..f82315254 100644 --- a/lib/src/io/vm.dart +++ b/lib/src/io/vm.dart @@ -87,6 +87,10 @@ DateTime modificationTime(String path) { String? getEnvironmentVariable(String name) => io.Platform.environment[name]; +void ensureWatchWillExit() { + if (!io.stdin.hasTerminal) io.stdin.listen(null, onDone: () => io.exit(0)); +} + Future> watchDir(String path, {bool poll = false}) async { var watcher = poll ? PollingDirectoryWatcher(path) : DirectoryWatcher(path); From b77bd29f4fdf5f3185a319c557ba7f76ba7dcfa5 Mon Sep 17 00:00:00 2001 From: Natalie Weizenbaum Date: Fri, 25 Mar 2022 18:16:04 -0700 Subject: [PATCH 2/4] Wait for stdin to close in a more async-API-friendly way Specifically, we now close the watcher when stdin closes, using the CancelableOperation API to represent both "a watcher that can be closed" and "a subscription to stdin". --- bin/sass.dart | 5 +- lib/src/executable/watch.dart | 127 +++++++++++++++++++--------------- lib/src/io/interface.dart | 14 ++-- lib/src/io/node.dart | 7 +- lib/src/io/vm.dart | 6 +- lib/src/utils.dart | 17 +++++ pubspec.yaml | 5 +- 7 files changed, 111 insertions(+), 70 deletions(-) diff --git a/bin/sass.dart b/bin/sass.dart index 20f6fbdf5..13b17ea53 100644 --- a/bin/sass.dart +++ b/bin/sass.dart @@ -4,6 +4,7 @@ import 'dart:isolate'; +import 'package:async/async.dart'; import 'package:path/path.dart' as p; import 'package:stack_trace/stack_trace.dart'; import 'package:term_glyph/term_glyph.dart' as term_glyph; @@ -55,8 +56,8 @@ Future main(List args) async { var graph = StylesheetGraph( ImportCache(loadPaths: options.loadPaths, logger: options.logger)); if (options.watch) { - ensureWatchWillExit(); - await watch(options, graph); + await CancelableOperation.race([onStdinClose(), watch(options, graph)]) + .valueOrCancellation(); return; } diff --git a/lib/src/executable/watch.dart b/lib/src/executable/watch.dart index 8989ab50e..1790a2222 100644 --- a/lib/src/executable/watch.dart +++ b/lib/src/executable/watch.dart @@ -2,8 +2,10 @@ // MIT-style license that can be found in the LICENSE file or at // https://opensource.org/licenses/MIT. +import 'dart:async'; import 'dart:collection'; +import 'package:async/async.dart'; import 'package:path/path.dart' as p; import 'package:stack_trace/stack_trace.dart'; import 'package:stream_transform/stream_transform.dart'; @@ -19,41 +21,46 @@ import 'compile_stylesheet.dart'; import 'options.dart'; /// Watches all the files in [graph] for changes and updates them as necessary. -Future watch(ExecutableOptions options, StylesheetGraph graph) async { - var directoriesToWatch = [ - ..._sourceDirectoriesToDestinations(options).keys, - for (var dir in _sourcesToDestinations(options).keys) p.dirname(dir), - ...options.loadPaths - ]; - - var dirWatcher = MultiDirWatcher(poll: options.poll); - await Future.wait(directoriesToWatch.map((dir) { - // If a directory doesn't exist, watch its parent directory so that we're - // notified once it starts existing. - while (!dirExists(dir)) { - dir = p.dirname(dir); - } - return dirWatcher.watch(dir); - })); - - // Before we start paying attention to changes, compile all the stylesheets as - // they currently exist. This ensures that changes that come in update a - // known-good state. - var watcher = _Watcher(options, graph); - for (var entry in _sourcesToDestinations(options).entries) { - graph.addCanonical(FilesystemImporter('.'), - p.toUri(canonicalize(entry.key)), p.toUri(entry.key), - recanonicalize: false); - var success = - await watcher.compile(entry.key, entry.value, ifModified: true); - if (!success && options.stopOnError) { - dirWatcher.events.listen(null).cancel(); - return; +///z +/// Canceling the operation closes the watcher. +CancelableOperation watch( + ExecutableOptions options, StylesheetGraph graph) { + return unwrapCancelableOperation(() async { + var directoriesToWatch = [ + ..._sourceDirectoriesToDestinations(options).keys, + for (var dir in _sourcesToDestinations(options).keys) p.dirname(dir), + ...options.loadPaths + ]; + + var dirWatcher = MultiDirWatcher(poll: options.poll); + await Future.wait(directoriesToWatch.map((dir) { + // If a directory doesn't exist, watch its parent directory so that we're + // notified once it starts existing. + while (!dirExists(dir)) { + dir = p.dirname(dir); + } + return dirWatcher.watch(dir); + })); + + // Before we start paying attention to changes, compile all the stylesheets as + // they currently exist. This ensures that changes that come in update a + // known-good state. + var watcher = _Watcher(options, graph); + for (var entry in _sourcesToDestinations(options).entries) { + graph.addCanonical(FilesystemImporter('.'), + p.toUri(canonicalize(entry.key)), p.toUri(entry.key), + recanonicalize: false); + var success = + await watcher.compile(entry.key, entry.value, ifModified: true); + if (!success && options.stopOnError) { + dirWatcher.events.listen(null).cancel(); + return CancelableOperation.fromFuture(Future.value()); + } } - } - print("Sass is watching for changes. Press Ctrl-C to stop.\n"); - await watcher.watch(dirWatcher); + print("Sass is watching for changes. Press Ctrl-C to stop.\n"); + return watcher.watch(dirWatcher); + }()); } /// Holds state that's shared across functions that react to changes on the @@ -124,31 +131,39 @@ class _Watcher { /// Listens to `watcher.events` and updates the filesystem accordingly. /// - /// Returns a future that will only complete if an unexpected error occurs. - Future watch(MultiDirWatcher watcher) async { - await for (var event in _debounceEvents(watcher.events)) { - var extension = p.extension(event.path); - if (extension != '.sass' && extension != '.scss' && extension != '.css') { - continue; - } + /// Returns an operation that will only complete if an unexpected error occurs + /// (or if a complation error occurs and `--stop-on-error` is passed). This + /// operation can be cancelled to close the watcher. + CancelableOperation watch(MultiDirWatcher watcher) { + StreamSubscription? subscription; + return CancelableOperation.fromFuture(() async { + subscription = _debounceEvents(watcher.events).listen(null); + await for (var event in SubscriptionStream(subscription!)) { + var extension = p.extension(event.path); + if (extension != '.sass' && + extension != '.scss' && + extension != '.css') { + continue; + } - switch (event.type) { - case ChangeType.MODIFY: - var success = await _handleModify(event.path); - if (!success && _options.stopOnError) return; - break; - - case ChangeType.ADD: - var success = await _handleAdd(event.path); - if (!success && _options.stopOnError) return; - break; - - case ChangeType.REMOVE: - var success = await _handleRemove(event.path); - if (!success && _options.stopOnError) return; - break; + switch (event.type) { + case ChangeType.MODIFY: + var success = await _handleModify(event.path); + if (!success && _options.stopOnError) return; + break; + + case ChangeType.ADD: + var success = await _handleAdd(event.path); + if (!success && _options.stopOnError) return; + break; + + case ChangeType.REMOVE: + var success = await _handleRemove(event.path); + if (!success && _options.stopOnError) return; + break; + } } - } + }(), onCancel: () => subscription?.cancel()); } /// Handles a modify event for the stylesheet at [path]. diff --git a/lib/src/io/interface.dart b/lib/src/io/interface.dart index 4784b0d41..f25b6cc3e 100644 --- a/lib/src/io/interface.dart +++ b/lib/src/io/interface.dart @@ -2,6 +2,7 @@ // MIT-style license that can be found in the LICENSE file or at // https://opensource.org/licenses/MIT. +import 'package:async/async.dart'; import 'package:watcher/watcher.dart'; /// An output sink that writes to this process's standard error. @@ -94,13 +95,14 @@ String? getEnvironmentVariable(String name) => throw ''; int get exitCode => throw ''; set exitCode(int value) => throw ''; -/// Attaches a listener to exit when stdin closes. +/// If stdin is a TTY, returns a [CancelableOperation] that completes once it +/// closes. /// -/// The listener is *not* attached when stdin is a TTY because it would -/// interfere with the Unix background job system. If we read from stdin and -/// then Ctrl+Z to move the process to the background, it will incorrectly -/// cause the job to stop. See: https://github.com/brunch/brunch/issues/998. -void ensureWatchWillExit() => throw ''; +/// Otherwise, returns a [CancelableOperation] that never completes. +/// +/// As long as this is uncanceled, it will monopolize stdin so that nothing else +/// can read from it. +CancelableOperation onStdinClose() => throw ''; /// Recursively watches the directory at [path] for modifications. /// diff --git a/lib/src/io/node.dart b/lib/src/io/node.dart index 66ac77f8c..97a13b009 100644 --- a/lib/src/io/node.dart +++ b/lib/src/io/node.dart @@ -6,6 +6,7 @@ import 'dart:async'; import 'dart:convert'; import 'dart:js_util'; +import 'package:async/async.dart'; import 'package:js/js.dart'; import 'package:node_interop/fs.dart'; import 'package:node_interop/node_interop.dart'; @@ -215,10 +216,12 @@ int get exitCode => process.exitCode; set exitCode(int code) => process.exitCode = code; -void ensureWatchWillExit() { +CancelableOperation onStdinClose() { + var completer = CancelableCompleter(); if (isStdinTTY == true) { - process.stdin.on('end', allowInterop(() => process.exit(0))); + process.stdin.on('end', allowInterop(() => completer.complete())); } + return completer.operation; } Future> watchDir(String path, {bool poll = false}) { diff --git a/lib/src/io/vm.dart b/lib/src/io/vm.dart index 52f209a20..72d74fde4 100644 --- a/lib/src/io/vm.dart +++ b/lib/src/io/vm.dart @@ -90,9 +90,9 @@ DateTime modificationTime(String path) { String? getEnvironmentVariable(String name) => io.Platform.environment[name]; -void ensureWatchWillExit() { - if (!io.stdin.hasTerminal) io.stdin.listen(null, onDone: () => io.exit(0)); -} +CancelableOperation onStdinClose() => io.stdin.hasTerminal + ? CancelableOperation.fromSubscription(io.stdin.listen(null)) + : CancelableCompleter().operation; Future> watchDir(String path, {bool poll = false}) async { var watcher = poll ? PollingDirectoryWatcher(path) : DirectoryWatcher(path); diff --git a/lib/src/utils.dart b/lib/src/utils.dart index 07f7fc046..3542a9cba 100644 --- a/lib/src/utils.dart +++ b/lib/src/utils.dart @@ -4,6 +4,7 @@ import 'dart:math' as math; +import 'package:async/async.dart'; import 'package:charcode/charcode.dart'; import 'package:collection/collection.dart'; import 'package:source_span/source_span.dart'; @@ -379,6 +380,22 @@ Future putIfAbsentAsync( return value; } +/// Unwraps a [Future] that wraps a [CancelableOperation]. +/// +/// If the returned operation is cancelled, it will cancel the inner operation +/// as soon as the future completes. +CancelableOperation unwrapCancelableOperation( + Future> future) { + var completer = CancelableCompleter( + onCancel: () => future.then((operation) => operation.cancel())); + + future.then((operation) { + operation.then(completer.complete, onError: completer.completeError); + }, onError: completer.completeError); + + return completer.operation; +} + /// Returns a deep copy of a map that contains maps. Map> copyMapOfMap(Map> map) => {for (var entry in map.entries) entry.key: Map.of(entry.value)}; diff --git a/pubspec.yaml b/pubspec.yaml index 28310a693..d9be3856d 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -12,7 +12,7 @@ environment: dependencies: args: ^2.0.0 - async: ^2.5.0 + async: ^2.9.0 charcode: ^1.2.0 cli_repl: ^0.2.1 collection: ^1.15.0 @@ -48,3 +48,6 @@ dev_dependencies: test_descriptor: ^2.0.0 test_process: ^2.0.0 yaml: ^3.1.0 + +dependency_overrides: + async: {git: https://github.com/dart-lang/async} From 202149fb998653db13d3c94e5e490f31cba35ac0 Mon Sep 17 00:00:00 2001 From: Natalie Weizenbaum Date: Tue, 5 Apr 2022 17:19:22 -0700 Subject: [PATCH 3/4] Remove dependency override --- pubspec.yaml | 3 --- 1 file changed, 3 deletions(-) diff --git a/pubspec.yaml b/pubspec.yaml index d9be3856d..aa749587e 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -48,6 +48,3 @@ dev_dependencies: test_descriptor: ^2.0.0 test_process: ^2.0.0 yaml: ^3.1.0 - -dependency_overrides: - async: {git: https://github.com/dart-lang/async} From cdef8fef94d923dc363d159eee1bd43803bcd85f Mon Sep 17 00:00:00 2001 From: Natalie Weizenbaum Date: Wed, 6 Apr 2022 13:35:07 -0700 Subject: [PATCH 4/4] Add a CHANGELOG entry --- CHANGELOG.md | 7 ++++++- pubspec.yaml | 2 +- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 95b472e87..8b090a20b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,9 @@ -## 1.49.12 +## 1.50.0 + +### Command Line Interface + +* Closing the standard input stream will now cause the `--watch` command to stop + running. ### Embedded Sass diff --git a/pubspec.yaml b/pubspec.yaml index aa390c4f2..99a6d535f 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -1,5 +1,5 @@ name: sass -version: 1.49.12-dev +version: 1.50.0-dev description: A Sass implementation in Dart. homepage: https://github.com/sass/dart-sass