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

(fix): ShowOutput flag displays in realtime #405

Merged
merged 2 commits into from
Nov 29, 2023

Conversation

hopefulTex
Copy link
Contributor

Fixes the --show-output flag not piping output.
Fixes the --show-output flag not showing live output.

Fixes the `--show-output` flag not piping output.
Fixes the `--show-output` flag not showing live output.
Added comments to explain output logic
@hopefulTex hopefulTex changed the title (fix): ShowOutput flag works as intended (fix): ShowOutput flag displays in realtime Aug 3, 2023
@@ -26,6 +26,7 @@ func (o Options) Run() error {
title: o.TitleStyle.ToLipgloss().Render(o.Title),
command: o.Command,
align: o.Align,
showOutput: o.ShowOutput && isTTY,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still missing in v0.11. It prevents from using spin command :(

@joshmedeski
Copy link

Please merge this, it's blocking my team 🙏

@maaslalani maaslalani merged commit 5c65944 into charmbracelet:main Nov 29, 2023
6 checks passed
@maaslalani
Copy link
Contributor

Sorry about the late merge, thanks for the reminder @joshmedeski!

@joshmedeski
Copy link

Just a heads up, this is still not working for me and a friend of mine. Should I open a new ticket to continue this work?

@hopefulTex
Copy link
Contributor Author

@joshmedeski does it break, or is it as though the feature was never added back?

@joshmedeski
Copy link

As though the feature was never added back.

I tested on main and it's working (see #453 (comment))

@hopefulTex
Copy link
Contributor Author

hopefulTex commented Dec 7, 2023

@joshmedeski I suggest opening a new ticket, just to keep what's been fixed and what hasn't been in order.
edit: could you please tag me in the ticket?

@hopefulTex
Copy link
Contributor Author

@joshmedeski Just tested the main branch and everything appears to be working as intended.
What usage is giving you trouble?
Does it work for other programs such as ping -c 4 example.com?
Are you using it in conjunction with the --timeout flag?

@hopefulTex hopefulTex deleted the patch-1 branch December 8, 2023 00:49
@joshmedeski
Copy link

My examples were setting output to a variable then echoing it later.

This is a simple example with fish:

set THING (gum spin --show-output -- ping -c 4 example.com) && echo $THING

It's working on main so hopefully on the next release the fix shows up properly, I just pulled the source code for now.

@pulberg
Copy link

pulberg commented Dec 9, 2023

Due to corp policies I am unable to pull from main or install via go, We use an internally referenced homebrew, any ETA on the next gum release that includes this fix? Thanks.

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