From 753c17eba4ceb2393f96be332bcc689fc92c3c5b Mon Sep 17 00:00:00 2001 From: Natalie Weizenbaum Date: Fri, 20 May 2022 13:20:24 -0700 Subject: [PATCH] Revert "Abort sass if stdin is closed when watching (#1411)" This reverts commit c7ab426cb049f4edd41339f8e3e45dd334ce380a. See #1665, #1411 --- CHANGELOG.md | 8 +++ bin/sass.dart | 4 +- lib/src/executable/watch.dart | 127 +++++++++++++++------------------- lib/src/io/interface.dart | 10 --- lib/src/io/node.dart | 12 ---- lib/src/io/vm.dart | 4 -- lib/src/utils.dart | 17 ----- pkg/sass_api/CHANGELOG.md | 4 ++ pkg/sass_api/pubspec.yaml | 4 +- pubspec.yaml | 4 +- 10 files changed, 73 insertions(+), 121 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b33341346..0821c016f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,11 @@ +## 1.52.1 + +### Command Line Interface + +* Fix a bug where `--watch` mode would close immediately in TTY mode. This was + caused by our change to close `--watch` when stdin was closed *outside of* TTY + mode, which has been reverted for now while we work on a fix. + ## 1.52.0 * Add support for arbitrary modifiers at the end of plain CSS imports, in diff --git a/bin/sass.dart b/bin/sass.dart index 13b17ea53..a95ad65ba 100644 --- a/bin/sass.dart +++ b/bin/sass.dart @@ -4,7 +4,6 @@ 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; @@ -56,8 +55,7 @@ Future main(List args) async { var graph = StylesheetGraph( ImportCache(loadPaths: options.loadPaths, logger: options.logger)); if (options.watch) { - await CancelableOperation.race([onStdinClose(), watch(options, graph)]) - .valueOrCancellation(); + await watch(options, graph); return; } diff --git a/lib/src/executable/watch.dart b/lib/src/executable/watch.dart index 1790a2222..8989ab50e 100644 --- a/lib/src/executable/watch.dart +++ b/lib/src/executable/watch.dart @@ -2,10 +2,8 @@ // 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'; @@ -21,46 +19,41 @@ import 'compile_stylesheet.dart'; import 'options.dart'; /// Watches all the files in [graph] for changes and updates them as necessary. -///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()); - } +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; + } + } - print("Sass is watching for changes. Press Ctrl-C to stop.\n"); - return watcher.watch(dirWatcher); - }()); + print("Sass is watching for changes. Press Ctrl-C to stop.\n"); + await watcher.watch(dirWatcher); } /// Holds state that's shared across functions that react to changes on the @@ -131,39 +124,31 @@ class _Watcher { /// Listens to `watcher.events` and updates the filesystem accordingly. /// - /// 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; - } + /// 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; + } - 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 f25b6cc3e..0ee35bc53 100644 --- a/lib/src/io/interface.dart +++ b/lib/src/io/interface.dart @@ -2,7 +2,6 @@ // 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. @@ -95,15 +94,6 @@ String? getEnvironmentVariable(String name) => throw ''; int get exitCode => throw ''; set exitCode(int value) => throw ''; -/// If stdin is a TTY, returns a [CancelableOperation] that completes once it -/// closes. -/// -/// 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. /// /// 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 97a13b009..12e24d5a6 100644 --- a/lib/src/io/node.dart +++ b/lib/src/io/node.dart @@ -6,7 +6,6 @@ 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'; @@ -196,9 +195,6 @@ 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'; @@ -216,14 +212,6 @@ int get exitCode => process.exitCode; set exitCode(int code) => process.exitCode = code; -CancelableOperation onStdinClose() { - var completer = CancelableCompleter(); - if (isStdinTTY == true) { - process.stdin.on('end', allowInterop(() => completer.complete())); - } - return completer.operation; -} - 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 72d74fde4..bdc89916b 100644 --- a/lib/src/io/vm.dart +++ b/lib/src/io/vm.dart @@ -90,10 +90,6 @@ DateTime modificationTime(String path) { String? getEnvironmentVariable(String name) => io.Platform.environment[name]; -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 3542a9cba..07f7fc046 100644 --- a/lib/src/utils.dart +++ b/lib/src/utils.dart @@ -4,7 +4,6 @@ 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'; @@ -380,22 +379,6 @@ 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/pkg/sass_api/CHANGELOG.md b/pkg/sass_api/CHANGELOG.md index b67329b84..ba47a2174 100644 --- a/pkg/sass_api/CHANGELOG.md +++ b/pkg/sass_api/CHANGELOG.md @@ -1,3 +1,7 @@ +## 1.0.0-beta.46 + +* No user-visible changes. + ## 1.0.0-beta.45 * **Breaking change:** Replace `StaticImport.supports` and `StaticImport.media` diff --git a/pkg/sass_api/pubspec.yaml b/pkg/sass_api/pubspec.yaml index 528c96904..4ed7a9142 100644 --- a/pkg/sass_api/pubspec.yaml +++ b/pkg/sass_api/pubspec.yaml @@ -2,7 +2,7 @@ name: sass_api # Note: Every time we add a new Sass AST node, we need to bump the *major* # version because it's a breaking change for anyone who's implementing the # visitor interface(s). -version: 1.0.0-beta.45 +version: 1.0.0-beta.46 description: Additional APIs for Dart Sass. homepage: https://github.com/sass/dart-sass @@ -10,7 +10,7 @@ environment: sdk: '>=2.12.0 <3.0.0' dependencies: - sass: 1.52.0 + sass: 1.52.1 dev_dependencies: dartdoc: ^5.0.0 diff --git a/pubspec.yaml b/pubspec.yaml index 013ec6ff0..4d9cd2da3 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -1,5 +1,5 @@ name: sass -version: 1.52.0 +version: 1.52.1 description: A Sass implementation in Dart. homepage: https://github.com/sass/dart-sass @@ -12,7 +12,7 @@ environment: dependencies: args: ^2.0.0 - async: ^2.9.0 + async: ^2.5.0 charcode: ^1.2.0 cli_repl: ^0.2.1 collection: ^1.15.0