Skip to content

Conversation

MTHLily
Copy link

@MTHLily MTHLily commented Jan 16, 2025

Fixes #800

Changes

  • Add --interactive/-i flag to restore previous functionality to select a table row then pipe it into another command

@MTHLily MTHLily requested a review from a team as a code owner January 16, 2025 03:38
@MTHLily MTHLily requested review from csandeep and removed request for a team January 16, 2025 03:38
table/command.go Outdated
@@ -110,7 +110,7 @@ func (o Options) Run() error {
rows = append(rows, table.Row(data[row]))
}

if o.Print || !term.IsTerminal(os.Stdout.Fd()) {
if !o.Interactive && (o.Print || !term.IsTerminal(os.Stdout.Fd())) {
Copy link
Member

Choose a reason for hiding this comment

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

I think maybe we should check if stdin is a terminal instead 🤔

Copy link
Author

Choose a reason for hiding this comment

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

Implemented your suggestions, @caarlos0. However, the behavior is inconsistent for the following cases:

results="$(gum table --height 10 -f test.csv)" # Results in an interactive table
results="$(gum table --height 10 <test.csv)" # Results in a printed table
results="$(cat test.csv | gum table --height 10)" # Results in a printed table

Copy link
Author

Choose a reason for hiding this comment

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

Reflecting on the way gum table is meant to be used and described in the readme, i.e. as a row selector for tabular data, I'm thinking that instead of adding the forced interactive flag, we ought to revert the original commit from #762. I don't think it's expected behavior for gum table to print a table instead of letting us choose a row.

Cards on the table, I was also using gum table to select row data and piping it to another program for my own script. So, I am a bit biased in this regard.

Copy link
Member

Choose a reason for hiding this comment

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

yes, in retrospect, I think that it makes sense. will do it

Copy link
Member

Choose a reason for hiding this comment

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

@bashbunni
Copy link
Contributor

Hey, looks like this was closed in #811. Thanks so much for the contribution :) If you have any questions or concerns about this PR, please let us know and we can reopen.

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.

gum table doesn't allow selecting on stdout redirect
4 participants