Revert "Abort sass if stdin is closed when watching (#1411)" (#1699)

This reverts commit c7ab426.

See #1665, #1411

Author: nex3

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

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<void> main(List<String> 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;
     }
 

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<void> 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<void>.value());
-      }
+Future<void> 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<void> watch(MultiDirWatcher watcher) {
-    StreamSubscription<WatchEvent>? subscription;
-    return CancelableOperation<void>.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<void> 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].

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<void> onStdinClose() => throw '';
-
 /// Recursively watches the directory at [path] for modifications.
 ///
 /// Returns a future that completes with a single-subscription stream once the

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<void> onStdinClose() {
-  var completer = CancelableCompleter<void>();
-  if (isStdinTTY == true) {
-    process.stdin.on('end', allowInterop(() => completer.complete()));
-  }
-  return completer.operation;
-}
-
 Future<Stream<WatchEvent>> watchDir(String path, {bool poll = false}) {
   var watcher = chokidar.watch(
       path, ChokidarOptions(disableGlobbing: true, usePolling: poll));

lib/src/io/vm.dart

@@ -90,10 +90,6 @@ DateTime modificationTime(String path) {
 
 String? getEnvironmentVariable(String name) => io.Platform.environment[name];
 
-CancelableOperation<void> onStdinClose() => io.stdin.hasTerminal
-    ? CancelableOperation.fromSubscription(io.stdin.listen(null))
-    : CancelableCompleter<void>().operation;
-
 Future<Stream<WatchEvent>> watchDir(String path, {bool poll = false}) async {
   var watcher = poll ? PollingDirectoryWatcher(path) : DirectoryWatcher(path);
 

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<V> putIfAbsentAsync<K, V>(
   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<T> unwrapCancelableOperation<T>(
-    Future<CancelableOperation<T>> future) {
-  var completer = CancelableCompleter<T>(
-      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<K1, Map<K2, V>> copyMapOfMap<K1, K2, V>(Map<K1, Map<K2, V>> map) =>
     {for (var entry in map.entries) entry.key: Map.of(entry.value)};

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`

pkg/sass_api/pubspec.yaml

@@ -2,15 +2,15 @@ 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
 
 environment:
   sdk: '>=2.12.0 <3.0.0'
 
 dependencies:
-  sass: 1.52.0
+  sass: 1.52.1
 
 dev_dependencies:
   dartdoc: ^5.0.0

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