CanastaWiki / Canasta-CLI

The Canasta command line interface, written in Go
MIT License
5 stars 14 forks source link

Provided Real-Time Output for External Subcommands #75

Closed chl178 closed 7 months ago

chl178 commented 1 year ago

Fixes #73

This updated version of the execute package adds a custom writerWithPrint struct that implements the io.Writer interface. This allows the output of the executed command to be printed to the console while also being captured in a buffer.

The writerWithPrint struct has a buf field of type bytes.Buffer. The Write method of the struct uses logging.Print function to print and writes the bytes to the buffer. So when we have verbose, it will provide the output of external commands in real time.

Test

I have tested all external subcommands including docker-compose, git, etc. When we have verbose, the new 'Run' function is able to successfully provide their real-time output. If there are bugs during their execution, the function will print the output of external subcommands we captured as before.

Please review and let me know if you have any questions or concerns.

Thank you.

jeffw16 commented 1 year ago

Best of luck with your finals! Please don't reply until you have time.

May I ask why we are using a multithreaded implementation with locks here? It seems like overkill for the purposes of the CLI (although I think it might help with all of the logging happening within the Canasta image). Locks take time to lock and unlock. If we are running a single-threaded process, there is no point in locking our writes to stdout.

jeffw16 commented 1 year ago

Also, forgive me if it turns out that MultiWriter is the de facto generic implementation of the io.Writer interface. I don't write Go, nor am I a huge fan of the language.

chl178 commented 1 year ago

@jeffw16 Thank you for your suggestion, you were correct. In a single-threaded process, there is no need to lock.

chl178 commented 1 year ago

@jeffw16 @yaronkoren @amalpaul54111 would you please take a look at this?

jeffw16 commented 1 year ago

The code itself looks good, but I don't have time to try it just yet.