piotrmurach / tty-spinner

A terminal spinner for tasks that have non-deterministic time frame.
https://ttytoolkit.org
MIT License
424 stars 27 forks source link

Spinner breaks on multiline message #43

Open d4rky-pl opened 3 years ago

d4rky-pl commented 3 years ago

Describe the problem

Right now tty-spinner does not support multiline messages so this code will result in the spinner continuously outputting to the terminal:

require 'tty-spinner'
spinner = TTY::Spinner.new("abc\ndef")
spinner.run do
  sleep 5
end

It would be great if the spinner was smart enough to check the number of the lines of the message and moved the cursor accordingly when cleaning.

An example use case for this would be to keep the output of the command from tty-command on the screen without making it into an output salad:

require 'tty-spinner'
require 'tty-command'

spinner = TTY::Spinner.new("Running command\n:output")

cmd = TTY::Command.new
cmd.run("yarn") do |out, err|
  buffer << out if out
  buffer << err if err
  spinner.update(output: buffer)
end
piotrmurach commented 3 years ago

Hi Michał 👋

Thanks for the issue report, and of course, big thank you for the sponsorship! ❤️

Supporting multiline content sounds good but there is a bit 'trickiness' involved in how the multi spinner works. I should have some time to investigate next week unless you have time to submit PR in the meantime?

I wonder if your use case wouldn't be better served by adding new api for logging as suggested in this issue. Any thoughts?

d4rky-pl commented 3 years ago

I tried looking into this and implementing this myself but I didn't manage to get far which is why I ultimately reported this as an issue. I'm not sure if I'll find time soon to look into this again but if I do I'll definitely send a PR :)

And yeah, I totally think the API for logging would solve my issue as well.

piotrmurach commented 3 years ago

There is a new log method in the master branch that handles printing multiline content above a spinner animation. Please give it a try. I changed your example to use this method. The notable change is that the TTY::Command instance doesn't log any output by providing :null printer. The logging in tty-command runs in a separate thread and would mingle with the spinner output. I also added auto_spin call to ensure animation during yarn install with a small delay.

require "tty-spinner"
require "tty-command"

spinner = TTY::Spinner.new("[:spinner] Running :cmd command", format: :bouncing_ball)
cmd = TTY::Command.new(printer: :null)

spinner.auto_spin
spinner.update(cmd: "yarn")

cmd.run("yarn") do |out, err|
  buffer = []
  buffer << out if out
  buffer << err if err
  spinner.log(buffer.join)
  sleep(0.5)
end

spinner.stop("Done")
d4rky-pl commented 3 years ago

@piotrmurach for starters - apologies for such a long delay in responding, I had so many other things that I had to implement into our CLI tool that I didn't get around to testing this.

Anyway - this sort of works, as in, it does output things properly for simple commands but it really hates cursor movement shell codes. If I just output the err as-is then it kind of works, just misses several lines. If I push it to the buffer and output the buffer like you proposed then it results in the same lines showing up multiple times.

Here's a simple repro repository: https://github.com/d4rky-pl/tty-spinner-log-test

My use-case here is a tool that wraps docker and docker-compose and sets up our local development environment and I wanted to output whatever the tools setting up things are doing. Unfortunately my knowledge of shell magic is painfully lacking so I'm afraid I have no idea if this an be resolved anyhow or if I should just strip the codes and deduplicate lines manually (or just drop the idea altogether and make --verbose` flag disable the spinner).

piotrmurach commented 2 years ago

@d4rky-pl This is well overdue! In future please ping me if you don't hear from me.

I looked into your docker-compose example. The output from docker-compose produces a lot of escape codes that, for example, move the cursor up and down the terminal to update output in place similar to TTY::Spinner::Multi. Any extra character such as a newline will throw the output off and make it look broken.

Now, the spinner log method is rather simple so I was rather surprised for it to not work correctly and mingle the output. The culprit turns out to be this line

write("#{cleared_text}#{"\n" unless cleared_text.end_with?("\n")}", false)

In particular, the bit that adds a new line when it's missing. So when I replaced the above with:

write(cleared_text, false)

and removed the buffering from your example,

cmd.run("docker-compose up") do |out, err|
  spinner.log(out) if out
  spinner.log(err) if err
end

the output from the docker-compose was correct.

Why the log method didn't work with the new line appending? As it turns out docker-compose adds, for better or worse, escape codes after the newline.

So given the above how should the log method work?

capripot commented 1 year ago

Probably most of users will appreciate having a new line added automatically.

I think best option is an option that would disable new line: trailing_newline: false. Option is true by default.

cmd.run("docker-compose up") do |out, err|
  spinner.log(out, trailing_newline: false) if out
  spinner.log(err, trailing_newline: false) if err
end