Skip to content

External print#235

Merged
gwenn merged 59 commits intokkawakam:masterfrom
gwenn:external-print
Mar 5, 2022
Merged

External print#235
gwenn merged 59 commits intokkawakam:masterfrom
gwenn:external-print

Conversation

@gwenn
Copy link
Copy Markdown
Collaborator

@gwenn gwenn commented Jun 2, 2019

See #229

cargo run --example external_print

To do:

  • Windows platform (only one printer supported, and when the printer is dropped we still wait)
  • Check interaction with history search, completion, ...

@gwenn gwenn mentioned this pull request Jun 4, 2019
@gwenn gwenn changed the title First draft for external print External print Jun 10, 2019
@gwenn
Copy link
Copy Markdown
Collaborator Author

gwenn commented Jun 18, 2019

https://github.com/ghc/ghc/blob/master/libraries/base/cbits/inputReady.c#L325

nightmare. A Console Handle will appear to be ready
...

@flxo
Copy link
Copy Markdown

flxo commented Jan 7, 2021

I tried this branch and discovered the following: When using the printer from different threads, the blocking readline call somehow interferes with the std::io::Write implementation produced by create_external_printer.

On unix or windows ?

Sorry forgot to add this. I tested on a x86 Linux and Mac.

@gwenn
Copy link
Copy Markdown
Collaborator Author

gwenn commented Jan 7, 2021

I will try to have a look this week-end.

# Conflicts:
#	src/tty/unix.rs
#	src/tty/windows.rs
@gwenn
Copy link
Copy Markdown
Collaborator Author

gwenn commented Jan 9, 2021

@flxo Ok, we should not use a BufReader here. Because buffered lines are not "selectable" anymore...

TODO Fix, update windows code accordingly
@gwenn
Copy link
Copy Markdown
Collaborator Author

gwenn commented Jan 9, 2021

@flxo Your issue should have been fixed by 0a5f0a4 on unix platform.

@flxo
Copy link
Copy Markdown

flxo commented Jan 11, 2021

@gwenn Thanks for the update. I tried your patches and the issue is gone.

In my software I have a (hacky) std::io::Write impl around the rustyline::ExternalPrinter which allows me to use std::io::stdout and the rustyline interface dependant on the use case (iteractive or not).
Would it be possible to provide std::io::Write directly?

@gwenn
Copy link
Copy Markdown
Collaborator Author

gwenn commented Jan 11, 2021

@gwenn Thanks for the update. I tried your patches and the issue is gone.

In my software I have a (hacky) std::io::Write impl around the rustyline::ExternalPrinter which allows me to use std::io::stdout and the rustyline interface dependant on the use case (iteractive or not).
Would it be possible to provide std::io::Write directly?

You should be able to implement an ExternalPrinter that writes to stdout, no ? Like here

@flxo
Copy link
Copy Markdown

flxo commented Jan 12, 2021

@gwenn My idea is to do something like

impl<T> std::io::Write for T where T: ExternalPrinter {
   ...
}

The prints are some via the writeln! family and with the impl of std::io::Write I can decided whether to use std::io::Stdout for non interactive usage (there's no rustyline::Editor around) and the thing returned from create_external_printer

let out = if interactive {
  Box::new(rl.create_external_printer()?) as Box<dyn std::io::Write>
} else {
  Box::new(std::io::stdout()) as Box<dyn std::io::Write>
}

writeln!(out, "test");

@gwenn
Copy link
Copy Markdown
Collaborator Author

gwenn commented Jan 12, 2021

The ExternalPrinter is now message oriented (\n is not meaningful anymore).
And Write is bytes/stream oriented.
So I would prefer that you implements Write on your own ExternalPrinter wrapper.

@bartlomieju
Copy link
Copy Markdown

bartlomieju commented Dec 17, 2021

Is there any chance this PR could be landed? We use rustyline for REPL in Deno and this feature would greatly improve the experience for streaming output (denoland/deno#8049), currently the output is interleaved with prompt line.

EDIT: Actually not a huge priority, we're gonna experiment with different crate for this purpose https://docs.rs/reedline

@gwenn
Copy link
Copy Markdown
Collaborator Author

gwenn commented Dec 17, 2021

Is there any chance this PR could be landed?

Current implementation is incomplete on windows.

EDIT: Actually not a huge priority, we're gonna experiment with different crate for this purpose https://docs.rs/reedline

#579

@vi
Copy link
Copy Markdown

vi commented Feb 25, 2022

Current implementation is incomplete on windows.

Can UNIX-only implementation land first then?

Shall an updated, but limited scope pull request (with ExternalPrinter cfg-gated) be submitted?

@gwenn
Copy link
Copy Markdown
Collaborator Author

gwenn commented Mar 3, 2022

Could you please give it another try (windows implementation should be fine now) ?
Thanks.

@gwenn gwenn merged commit ff5611f into kkawakam:master Mar 5, 2022
@gwenn gwenn deleted the external-print branch March 5, 2022 16:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants