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

nu-table: Improve table -a #11905

Merged

Conversation

zhiburt
Copy link
Contributor

@zhiburt zhiburt commented Feb 20, 2024

Hi there;

Sorry took that long to respond.

I guess it's good?
It will consume the whole stream whether possible.
I do believe it will be faster in WSL in general too (in a sense of whole buffer output), but its interesting issue probably needed to be separated. It was not very well explained as well.

> 0..2000 | table -a 2
╭───┬──────╮
 0     0 
 1     1 
 2  ...  
 3  1999 
 4  2000 
╰───┴──────╯

Take care

fix: #11845

cc: @fdncred

Consume the whole stream;
Make it more effective;
@fdncred
Copy link
Collaborator

fdncred commented Feb 20, 2024

@zhiburt for some reason this is breaking ci with things like these. Any ideas what's going on?

test commands::table::table_abbreviation has been running for over 60 seconds
test commands::table::table_abbreviation_by_config has been running for over 60 seconds

@fdncred
Copy link
Collaborator

fdncred commented Feb 23, 2024

@zhiburt something about these tests isn't quite right. any ideas?

@fdncred
Copy link
Collaborator

fdncred commented Feb 23, 2024

definitely closer.

@zhiburt
Copy link
Contributor Author

zhiburt commented Feb 23, 2024

Don't forget to squash commits.

Yes I was not sure about -a 0 and made it return empty result.

@fdncred
Copy link
Collaborator

fdncred commented Feb 23, 2024

thanks again @zhiburt!

@fdncred
Copy link
Collaborator

fdncred commented Feb 23, 2024

@zhiburt one more question. i just want to make sure this stream collect is only being called with -a, we don't want to be collecting on other table modes. can you confirm that is correct?

@zhiburt
Copy link
Contributor Author

zhiburt commented Feb 24, 2024

i just want to make sure this stream collect is only being called with -a, we don't want to be collecting on other table modes. can you confirm that is correct?

Correct

match self.cfg.abbreviation {
Some(abbr) => {
(batch, _, end) =
stream_collect_abbriviated(&mut self.stream, abbr, self.ctrlc.clone());
}
None => {
// Pull from stream until time runs out or we have enough items
(batch, end) =
stream_collect(&mut self.stream, STREAM_PAGE_SIZE, self.ctrlc.clone());
}
}

@fdncred
Copy link
Collaborator

fdncred commented Feb 24, 2024

Thanks!

@fdncred fdncred added the pr:language This PR makes some language changes label Feb 24, 2024
@fdncred fdncred merged commit 6be91a6 into nushell:main Feb 24, 2024
19 checks passed
@fdncred fdncred added pr:commands This PR changes our commands in some way pr:bugfix This PR fixes some bug and removed pr:language This PR makes some language changes labels Feb 24, 2024
@hustcer hustcer added this to the v0.91.0 milestone Feb 24, 2024
kik4444 pushed a commit to kik4444/nushell-fork that referenced this pull request Feb 28, 2024
Hi there;

Sorry took that long to respond.

I guess it's good?
It will consume the whole stream whether possible.
I do believe it will be faster in WSL in general too (in a sense of
whole buffer output), but its interesting issue probably needed to be
separated. It was not very well explained as well.

```nushell
> 0..2000 | table -a 2
╭───┬──────╮
│ 0 │    0 │
│ 1 │    1 │
│ 2 │ ...  │
│ 3 │ 1999 │
│ 4 │ 2000 │
╰───┴──────╯
```

Take care

fix: nushell#11845

cc: @fdncred
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr:bugfix This PR fixes some bug pr:commands This PR changes our commands in some way
Projects
None yet
Development

Successfully merging this pull request may close these issues.

table --abbreviated/-a doesn't work on large (1k+ row) or slow (even when collected) tables
3 participants