Skip to content
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

Add options to disable prefix and/or color on spawn output #47

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

eigilsagafos
Copy link

I'm using turbowatch in an environment where I need more control over the output from the spawned process. The prefix and color is giving me some unnecessary additional work, so I added option to toggle those options. Hope this makes sense.

@eigilsagafos
Copy link
Author

@gajus do you consider PRs for this project?

@gajus
Copy link
Owner

gajus commented Jun 2, 2023

I have not had a minute to evaluate the approach.

I need to figure out how to make this more universal.

Currently leaning towards exposing a method along the lines of onMessage that allows to filter/customize the output.

@virtuallyunknown
Copy link

virtuallyunknown commented Jun 13, 2023

If I may voice my personal opinion, I would love to have an option such as (or similar to) the prefixOutput in this PR. When I have turbowatch run a turbo command on top of it, it becomes a bit difficult to read, since the application output becomes prefixed twice:

a6480159 > @monorepo/package:command: hello world

@gajus
Copy link
Owner

gajus commented Jun 13, 2023

Just for my understanding, in what scenario would you run turbo on top of turbowatch?

@gajus
Copy link
Owner

gajus commented Jun 13, 2023

We've experimented with different forms of log prefixes in our setup, including removing the prefix, and removing the prefix scored the lowest sentiment. For two days all issue reports were useless because there was no good way to track down the origin of the error. Therefore, feels like a bad pattern to allow.

@gajus
Copy link
Owner

gajus commented Jun 13, 2023

I am planning to share a Twitter thread/article sometime next week about what setup we've landed on eventually. We have something that works great in terms of DX. However, it is definitely not going to be a great option for everyone because it makes heavy assumptions about the local setup (such as use of iTerm)

@gajus
Copy link
Owner

gajus commented Jun 13, 2023

Happy to merge this behind a configuration, not as a default though.

@virtuallyunknown
Copy link

virtuallyunknown commented Jun 13, 2023

Hey gajus. First, I just want to say thank you for developing this library.

I don't consider myself knowledgeable enough to provide an opinion on your most of what you commented, but if it's helpful in any way I can simply answer your question.

Just for my understanding, in what scenario would you run turbo on top of turbowatch?

EDIT: Removing my long reply here sorry about that. I am not running turbo on top of turbowatch, just realized I misused the phrase by accident.

@eigilsagafos
Copy link
Author

Thanks @gajus! Behind a flag would be great!

We have a dev server that manages several services as child processes and we centralize the logs with our own prefixes. So we are already in control and know the source. The prefix/color added noise that I had to strip away. (We also use a JSON logger so the lines will not parse with the prefix)

@robknight
Copy link

+1 to making this configurable.

@gajus It looks to me like this PR does what you suggested. Is it possible to merge it, or does it require further changes? I would be happy to contribute changes to get this done.

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.

4 participants