Skip to content

feat(table): Add interactive flag for #800#806

Closed
MTHLily wants to merge 2 commits intocharmbracelet:mainfrom
MTHLily:main
Closed

feat(table): Add interactive flag for #800#806
MTHLily wants to merge 2 commits intocharmbracelet:mainfrom
MTHLily:main

Conversation

@MTHLily
Copy link
Copy Markdown

@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
Comment thread table/command.go Outdated
}

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

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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Contributor

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
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@bashbunni
Copy link
Copy Markdown
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