New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make stdout unbuffered #1355

Merged
merged 1 commit into from Dec 23, 2018

Conversation

3 participants
@kt3k
Copy link
Contributor

kt3k commented Dec 15, 2018

This PR fixes #948. Continued from ry@7560787.

I copied //js/testing/* to //tools/ts_builder_library/testing/* (and slightly modified them to work with node.js) because //js/testing is not isomorphic anymore (doesn't work in (ts-)node because of import ... from "deno" line) and use it in ts_builder_library testing.

@kt3k kt3k changed the title Make stdout unbuffered WIP Make stdout unbuffered Dec 15, 2018

@kt3k kt3k force-pushed the kt3k:feature/unbuffered-stdout branch from b6042cd to 1b69eac Dec 15, 2018

@ry

This comment has been minimized.

Copy link
Collaborator

ry commented Dec 15, 2018

I've been hesitating on doing this because I think it's nice if we can maintain browser compatibility with the testing library.. It's such a minor thing to loose compatibility for.

One idea is to abuse the console.group() API for printing to the same line. Something like this:

console.group()
console.log("testing foobar....")
// ....
console.log("OK")
console.groupEnd()

Although printing to the same line isn't exactly what console.group is for, it's somewhat similar... It would allow us to still use testing in the browser.

@kt3k

This comment has been minimized.

Copy link
Contributor Author

kt3k commented Dec 15, 2018

@ry OK. I'll separate this PR into 2 pieces. line buffer related changes and testing library changes.

@justjavac

This comment has been minimized.

Copy link
Contributor

justjavac commented Dec 18, 2018

I think we should not use group, maybe we can choose groupCollapsed instead. and group can also be compatible with other platforms, such as Node.js/Browsers.

@ry

This comment has been minimized.

Copy link
Collaborator

ry commented Dec 18, 2018

yes - groupCollapsed seems more appropriate

@kt3k kt3k force-pushed the kt3k:feature/unbuffered-stdout branch 2 times, most recently from 3e7314a to 26b3600 Dec 18, 2018

@kt3k

This comment has been minimized.

Copy link
Contributor Author

kt3k commented Dec 19, 2018

  • Limitted this PR to stdout changes.
  • Added a test case for checking output without line break. (//tests/stdout.ts)

I still don't figure out how to create unbuffered stdout in windows.

let stdout = unsafe {std::fs::File::from_raw_handle(kernel32::GetStdHandle(winapi::winbase::STD_OUTPUT_HANDLE))};

m.insert(1, Repr::Stdout(
tokio::fs::File::from_std(stdout)));

This comment has been minimized.

@ry

ry Dec 19, 2018

Collaborator

consider rewriting like this

    m.insert(1, Repr::Stdout({
      #[cfg(not(windows))]
      let stdout = unsafe {std::fs::File::from_raw_fd(1)};
      #[cfg(windows)]
      let stdout = unsafe { std::fs::File::from_raw_handle(kernel32::GetStdHandle(winapi::winbase::STD_OUTPUT_HANDLE))};
      tokio::fs::File::from_std(stdout)
    }));

@kt3k kt3k force-pushed the kt3k:feature/unbuffered-stdout branch 2 times, most recently from 44c640e to b13f41b Dec 20, 2018

@kt3k kt3k changed the title WIP Make stdout unbuffered Make stdout unbuffered Dec 21, 2018

@kt3k

This comment has been minimized.

Copy link
Contributor Author

kt3k commented Dec 21, 2018

Now build and tests pass both in travis and appveyor. Please review.

@ry

This comment has been minimized.

Copy link
Collaborator

ry commented Dec 21, 2018

@kt3k Cool - looks good - sorry for the delay. Can you please apply the following clean ups:

From 62cd858fe927498c171e0954677a6beab5ceb0fd Mon Sep 17 00:00:00 2001
From: Ryan Dahl <ry@tinyclouds.org>
Date: Fri, 21 Dec 2018 12:53:59 -0500
Subject: [PATCH] renaming, formatting

---
 src/resources.rs                                   | 9 +++++++--
 tests/stdout.test                                  | 2 --
 tests/unbufferred_stdout.test                      | 2 ++
 tests/{stdout.ts => unbufferred_stdout.ts}         | 0
 tests/{stdout.ts.out => unbufferred_stdout.ts.out} | 0
 5 files changed, 9 insertions(+), 4 deletions(-)
 delete mode 100644 tests/stdout.test
 create mode 100644 tests/unbufferred_stdout.test
 rename tests/{stdout.ts => unbufferred_stdout.ts} (100%)
 rename tests/{stdout.ts.out => unbufferred_stdout.ts.out} (100%)

diff --git a/src/resources.rs b/src/resources.rs
index a69cae3..64ec959 100644
--- a/src/resources.rs
+++ b/src/resources.rs
@@ -65,13 +65,18 @@ lazy_static! {
 
     m.insert(1, Repr::Stdout({
       #[cfg(not(windows))]
-      let stdout = unsafe {std::fs::File::from_raw_fd(1)};
+      let stdout = unsafe { std::fs::File::from_raw_fd(1) };
       #[cfg(windows)]
-      let stdout = unsafe {std::fs::File::from_raw_handle(kernel32::GetStdHandle(winapi::um::winbase::STD_OUTPUT_HANDLE))};
+      let stdout = unsafe {
+        std::fs::File::from_raw_handle(kernel32::GetStdHandle(
+            winapi::um::winbase::STD_OUTPUT_HANDLE))
+      };
       tokio::fs::File::from_std(stdout)
     }));
 
+    // TODO Should stderr also be unbufferred?
     m.insert(2, Repr::Stderr(tokio::io::stderr()));
+
     m
   });
 }
diff --git a/tests/stdout.test b/tests/stdout.test
deleted file mode 100644
index c66f5c2..0000000
--- a/tests/stdout.test
+++ /dev/null
@@ -1,2 +0,0 @@
-args: tests/stdout.ts --reload
-output: tests/stdout.ts.out
diff --git a/tests/unbufferred_stdout.test b/tests/unbufferred_stdout.test
new file mode 100644
index 0000000..94475cf
--- /dev/null
+++ b/tests/unbufferred_stdout.test
@@ -0,0 +1,2 @@
+args: tests/unbufferred_stdout.ts --reload
+output: tests/unbufferred_stdout.ts.out
diff --git a/tests/stdout.ts b/tests/unbufferred_stdout.ts
similarity index 100%
rename from tests/stdout.ts
rename to tests/unbufferred_stdout.ts
diff --git a/tests/stdout.ts.out b/tests/unbufferred_stdout.ts.out
similarity index 100%
rename from tests/stdout.ts.out
rename to tests/unbufferred_stdout.ts.out
-- 
2.15.0

Please squash the commits and set yourself as author

@ry

This comment has been minimized.

Copy link
Collaborator

ry commented Dec 21, 2018

@kt3k I think we ought to also make stderr unbufferred now as well.

In the above patch I just realized I spelled "unbuffered" wrong with two "r"s. Please correct my mistakes.

feat: make stdout unbuffered
and added tests of unbuffered stdout/stderr

@kt3k kt3k force-pushed the kt3k:feature/unbuffered-stdout branch from b13f41b to 8d352ac Dec 23, 2018

@kt3k

This comment has been minimized.

Copy link
Contributor Author

kt3k commented Dec 23, 2018

@ry
Updated and squashed the commits.

After checking stderr in some versions of deno, it seems already unbuffered. I only added test of unbuffered_stderr and it seems passing in travis and appveyor without change of impl.

You can check it like:

deno https://raw.githubusercontent.com/denoland/deno/8d352ace7f52c6881dca649be20325586353e88f/tests/unbuffered_stderr.ts
@ry

ry approved these changes Dec 23, 2018

Copy link
Collaborator

ry left a comment

LGTM! Thanks - I’m looking forward to seeing those “...ok” on the same line :)

@ry ry merged commit bee55fc into denoland:master Dec 23, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
license/cla Contributor License Agreement is signed.
Details

@kt3k kt3k deleted the kt3k:feature/unbuffered-stdout branch Dec 23, 2018

ry added a commit to ry/deno that referenced this pull request Dec 23, 2018

v0.2.4
- Upgrade V8 to 7.2.502.16 (denoland#1403)
- Remove support for extensionless import  (denoland#1396)
- make stdout unbuffered (denoland#1355)
- Implement `Body.formData` for fetch (denoland#1393)
- Improve handling of non-coercable objects in assertEqual (denoland#1385)
- Avoid fetch segfault on empty Uri (denoland#1394)
- Expose deno.inspect (denoland#1378)
- "cargo build" support (denoland#1369 denoland#1296 denoland#1377 denoland#1379)
- Add illegal header name and value guards (denoland#1375)
- Fix URLSearchParams set() and constructor() (denoland#1368)
- Remove prebuilt v8 support (denoland#1369)
- Enable jumbo build in release. (denoland#1362)
- Add URL implementation (denoland#1359)
- Add console.count and console.time (denoland#1358)

@ry ry referenced this pull request Dec 23, 2018

Merged

v0.2.4 #1405

ry added a commit to ry/deno that referenced this pull request Dec 23, 2018

v0.2.4
- Upgrade V8 to 7.2.502.16 (denoland#1403)
- Remove support for extensionless import  (denoland#1396)
- make stdout unbuffered (denoland#1355)
- Implement `Body.formData` for fetch (denoland#1393)
- Improve handling of non-coercable objects in assertEqual (denoland#1385)
- Avoid fetch segfault on empty Uri (denoland#1394)
- Expose deno.inspect (denoland#1378)
- "cargo build" support (denoland#1369 denoland#1296 denoland#1377 denoland#1379)
- Add illegal header name and value guards (denoland#1375)
- Fix URLSearchParams set() and constructor() (denoland#1368)
- Remove prebuilt v8 support (denoland#1369)
- Enable jumbo build in release. (denoland#1362)
- Add URL implementation (denoland#1359)
- Add console.count and console.time (denoland#1358)

ry added a commit to ry/deno that referenced this pull request Dec 23, 2018

v0.2.4
- Upgrade V8 to 7.2.502.16 (denoland#1403)
- Remove support for extensionless import  (denoland#1396)
- make stdout unbuffered (denoland#1355)
- Implement `Body.formData` for fetch (denoland#1393)
- Improve handling of non-coercable objects in assertEqual (denoland#1385)
- Avoid fetch segfault on empty Uri (denoland#1394)
- Expose deno.inspect (denoland#1378)
- "cargo build" support (denoland#1369 denoland#1296 denoland#1377 denoland#1379)
- Add illegal header name and value guards (denoland#1375)
- Fix URLSearchParams set() and constructor() (denoland#1368)
- Remove prebuilt v8 support (denoland#1369)
- Enable jumbo build in release. (denoland#1362)
- Add URL implementation (denoland#1359)
- Add console.count and console.time (denoland#1358)

ry added a commit to ry/deno that referenced this pull request Dec 23, 2018

v0.2.4
- Upgrade V8 to 7.2.502.16 (denoland#1403)
- Remove support for extensionless import  (denoland#1396)
- make stdout unbuffered (denoland#1355)
- Implement `Body.formData` for fetch (denoland#1393)
- Improve handling of non-coercable objects in assertEqual (denoland#1385)
- Avoid fetch segfault on empty Uri (denoland#1394)
- Expose deno.inspect (denoland#1378)
- "cargo build" support (denoland#1369 denoland#1296 denoland#1377 denoland#1379)
- Add illegal header name and value guards (denoland#1375)
- Fix URLSearchParams set() and constructor() (denoland#1368)
- Remove prebuilt v8 support (denoland#1369)
- Enable jumbo build in release. (denoland#1362)
- Add URL implementation (denoland#1359)
- Add console.count and console.time (denoland#1358)

ry added a commit to ry/deno that referenced this pull request Dec 24, 2018

v0.2.4
- Upgrade V8 to 7.2.502.16 (denoland#1403)
- Remove support for extensionless import  (denoland#1396)
- make stdout unbuffered (denoland#1355)
- Implement `Body.formData` for fetch (denoland#1393)
- Improve handling of non-coercable objects in assertEqual (denoland#1385)
- Avoid fetch segfault on empty Uri (denoland#1394)
- Expose deno.inspect (denoland#1378)
- "cargo build" support (denoland#1369 denoland#1296 denoland#1377 denoland#1379)
- Add illegal header name and value guards (denoland#1375)
- Fix URLSearchParams set() and constructor() (denoland#1368)
- Remove prebuilt v8 support (denoland#1369)
- Enable jumbo build in release. (denoland#1362)
- Add URL implementation (denoland#1359)
- Add console.count and console.time (denoland#1358)

ry added a commit to ry/deno that referenced this pull request Dec 24, 2018

v0.2.4
- "cargo build" support (denoland#1369 denoland#1296 denoland#1377 denoland#1379)
- Remove support for extensionless import (denoland#1396)
- Upgrade V8 to 7.2.502.16 (denoland#1403)
- make stdout unbuffered (denoland#1355)
- Implement `Body.formData` for fetch (denoland#1393)
- Improve handling of non-coercable objects in assertEqual (denoland#1385)
- Avoid fetch segfault on empty Uri (denoland#1394)
- Expose deno.inspect (denoland#1378)
- Add illegal header name and value guards (denoland#1375)
- Fix URLSearchParams set() and constructor() (denoland#1368)
- Remove prebuilt v8 support (denoland#1369)
- Enable jumbo build in release. (denoland#1362)
- Add URL implementation (denoland#1359)
- Add console.count and console.time (denoland#1358)
- runtime arg check `URLSearchParams` (denoland#1390)

ry added a commit to ry/deno that referenced this pull request Dec 24, 2018

v0.2.4
- "cargo build" support (denoland#1369 denoland#1296 denoland#1377 denoland#1379)
- Remove support for extensionless import (denoland#1396)
- Upgrade V8 to 7.2.502.16 (denoland#1403)
- make stdout unbuffered (denoland#1355)
- Implement `Body.formData` for fetch (denoland#1393)
- Improve handling of non-coercable objects in assertEqual (denoland#1385)
- Avoid fetch segfault on empty Uri (denoland#1394)
- Expose deno.inspect (denoland#1378)
- Add illegal header name and value guards (denoland#1375)
- Fix URLSearchParams set() and constructor() (denoland#1368)
- Remove prebuilt v8 support (denoland#1369)
- Enable jumbo build in release. (denoland#1362)
- Add URL implementation (denoland#1359)
- Add console.count and console.time (denoland#1358)
- runtime arg check `URLSearchParams` (denoland#1390)

ry added a commit that referenced this pull request Dec 24, 2018

v0.2.4
- "cargo build" support (#1369 #1296 #1377 #1379)
- Remove support for extensionless import (#1396)
- Upgrade V8 to 7.2.502.16 (#1403)
- make stdout unbuffered (#1355)
- Implement `Body.formData` for fetch (#1393)
- Improve handling of non-coercable objects in assertEqual (#1385)
- Avoid fetch segfault on empty Uri (#1394)
- Expose deno.inspect (#1378)
- Add illegal header name and value guards (#1375)
- Fix URLSearchParams set() and constructor() (#1368)
- Remove prebuilt v8 support (#1369)
- Enable jumbo build in release. (#1362)
- Add URL implementation (#1359)
- Add console.count and console.time (#1358)
- runtime arg check `URLSearchParams` (#1390)

kt3k added a commit to kt3k/deno that referenced this pull request Jan 3, 2019

feat: implement console.groupCollapsed
This implementation of groupCollapsed is intentionally different
from the spec defined by whatwg. See the conversation in denoland#1355
and denoland#1363.

kt3k added a commit to kt3k/deno that referenced this pull request Jan 3, 2019

feat: implement console.groupCollapsed
This implementation of groupCollapsed is intentionally different
from the spec defined by whatwg. See the conversation in denoland#1355
and denoland#1363.

@kt3k kt3k referenced this pull request Jan 3, 2019

Merged

Implement console.groupCollapsed #1452

2 of 2 tasks complete

kt3k added a commit to kt3k/deno that referenced this pull request Jan 3, 2019

feat: implement console.groupCollapsed
This implementation of groupCollapsed is intentionally different
from the spec defined by whatwg. See the conversation in denoland#1355
and denoland#1363.

kt3k added a commit to kt3k/deno that referenced this pull request Jan 3, 2019

feat: implement console.groupCollapsed
This implementation of groupCollapsed is intentionally different
from the spec defined by whatwg. See the conversation in denoland#1355
and denoland#1363.

kt3k added a commit to kt3k/deno that referenced this pull request Jan 3, 2019

feat: implement console.groupCollapsed
This implementation of groupCollapsed is intentionally different
from the spec defined by whatwg. See the conversation in denoland#1355
and denoland#1363.

kt3k added a commit to kt3k/deno that referenced this pull request Jan 3, 2019

feat: implement console.groupCollapsed
This implementation of groupCollapsed is intentionally different
from the spec defined by whatwg. See the conversation in denoland#1355
and denoland#1363.

kt3k added a commit to kt3k/deno that referenced this pull request Jan 3, 2019

feat: implement console.groupCollapsed
This implementation of groupCollapsed is intentionally different
from the spec defined by whatwg. See the conversation in denoland#1355
and denoland#1363.

ry added a commit that referenced this pull request Jan 6, 2019

Implement console.groupCollapsed (#1452)
This implementation of groupCollapsed is intentionally different
from the spec defined by whatwg. See the conversation in #1355
and #1363.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment