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

from ssv is unable to detect empty columns #4182

Closed
waldyrious opened this issue Dec 5, 2021 · 6 comments
Closed

from ssv is unable to detect empty columns #4182

waldyrious opened this issue Dec 5, 2021 · 6 comments

Comments

@waldyrious
Copy link
Contributor

waldyrious commented Dec 5, 2021

Describe the bug

Using the example output from the docker ps documentation:

CONTAINER ID        IMAGE               COMMAND             CREATED             STATUS              PORTS               NAMES
673394ef1d4c        busybox             "top"               47 seconds ago      Up 45 seconds                           nostalgic_shockley
d85756f57265        busybox             "top"               52 seconds ago      Up 51 seconds                           high_albattani

Here's how from ssv parses it:

> cat ~/docker-ps.txt | from ssv
───┬──────────────┬─────────┬─────────┬────────────────┬───────────────┬────────────────────
 # │ CONTAINER ID │  IMAGE  │ COMMAND │    CREATED     │    STATUS     │       PORTS        
───┼──────────────┼─────────┼─────────┼────────────────┼───────────────┼────────────────────
 0 │ 673394ef1d4c │ busybox │ "top"   │ 47 seconds ago │ Up 45 seconds │ nostalgic_shockley 
 1 │ d85756f57265 │ busybox │ "top"   │ 52 seconds ago │ Up 51 seconds │ high_albattani     
───┴──────────────┴─────────┴─────────┴────────────────┴───────────────┴────────────────────

Note how the contents of the NAMES column were pulled into the PORTS column, which should be empty, and the NAMES column header doesn't even appear.

How to reproduce

  1. Create a ~/docker-ps.txt file with the contents indicated above
  2. Run ~/docker-ps.txt | from ssv

Expected behavior

The correct output should be:

> cat ~/docker-ps.txt | from ssv
───┬──────────────┬─────────┬─────────┬────────────────┬───────────────┬───────┬────────────────────
 # │ CONTAINER ID │  IMAGE  │ COMMAND │    CREATED     │    STATUS     │ PORTS │       NAMES        
───┼──────────────┼─────────┼─────────┼────────────────┼───────────────┼───────┼────────────────────
 0 │ 673394ef1d4c │ busybox │ "top"   │ 47 seconds ago │ Up 45 seconds │       │ nostalgic_shockley 
 1 │ d85756f57265 │ busybox │ "top"   │ 52 seconds ago │ Up 51 seconds │       │ high_albattani     
───┴──────────────┴─────────┴─────────┴────────────────┴───────────────┴───────┴────────────────────

Screenshots

No response

Configuration

key value
version 0.40.0
build_os linux-x86_64
rust_version rustc 1.56.1
cargo_version cargo 1.56.0
pkg_version 0.40.0
build_time 1980-01-01 00:00:00 +00:00
build_rust_channel release
features clipboard-cli, ctrlc, dataframe, default, rustyline, term, trash, uuid, which, zip
installed_plugins binaryview, chart bar, chart line, fetch, from bson, from sqlite, inc, match, post, ps, query json, s3, selector, start, sys, textview, to bson, to sqlite, tree, xpath

Additional context

If it helps, detect columns is able to identify the empty column:

> cat ~/docker-ps.txt | detect columns
───┬──────────────┬──────────────┬─────────┬─────────┬─────────┬─────────┬───────┬────────────────────
 # │  CONTAINER   │      ID      │  IMAGE  │ COMMAND │ CREATED │ STATUS  │ PORTS │       NAMES        
───┼──────────────┼──────────────┼─────────┼─────────┼─────────┼─────────┼───────┼────────────────────
 0 │ 673394ef1d4c │ 673394ef1d4c │ busybox │ "top"   │ seconds │ seconds │       │ nostalgic_shockley 
 1 │ d85756f57265 │ d85756f57265 │ busybox │ "top"   │ seconds │ seconds │       │ high_albattani     
───┴──────────────┴──────────────┴─────────┴─────────┴─────────┴─────────┴───────┴────────────────────

(There are other bugs in this output, but I'll report those separately. EDIT: Reported them as #4183.)

@sophiajt
Copy link
Contributor

sophiajt commented Dec 5, 2021

@waldyrious - out of curiosity, does the detect columns command do better here?

@waldyrious
Copy link
Contributor Author

waldyrious commented Dec 5, 2021

@jntrnr yes, but it has its own issues — see the "Additional context" section above :)

@fdncred
Copy link
Collaborator

fdncred commented Mar 12, 2022

This works | from ssv --aligned-columns in 0.59.1. Closing.

╭───┬──────────────┬─────────┬─────────┬────────────────┬───────────────┬───────┬────────────────────╮
│ # │ CONTAINER ID │  IMAGE  │ COMMAND │    CREATED     │    STATUS     │ PORTS │       NAMES        │
├───┼──────────────┼─────────┼─────────┼────────────────┼───────────────┼───────┼────────────────────┤
│ 0 │ 673394ef1d4c │ busybox │ "top"   │ 47 seconds ago │ Up 45 seconds │       │ nostalgic_shockley │
│ 1 │ d85756f57265 │ busybox │ "top"   │ 52 seconds ago │ Up 51 seconds │       │ high_albattani     │
├───┼──────────────┼─────────┼─────────┼────────────────┼───────────────┼───────┼────────────────────┤
│ # │ CONTAINER ID │  IMAGE  │ COMMAND │    CREATED     │    STATUS     │ PORTS │       NAMES        │
╰───┴──────────────┴─────────┴─────────┴────────────────┴───────────────┴───────┴────────────────────╯

@fdncred fdncred closed this as completed Mar 12, 2022
@waldyrious
Copy link
Contributor Author

Looks like this was a duplicate of #1350 after all. But as @kvrhdn commented there, it's not clear why --aligned-columns isn't the default behavior. I can see that it actually used to be the default, and the behavior was changed in #912 to fix #910, but honestly, when data is space-separated, isn't it more likely that it's column-aligned than not? I'd argue that the flag should be reversed, aligned columns should be the default, and opting out from that could be implemented with a flag like --unaligned-columns or something along those lines. WDYT @jntrnr?

@Caedan
Copy link
Contributor

Caedan commented Feb 1, 2023

Looks like this was a duplicate of #1350 after all. But as @kvrhdn commented there, it's not clear why --aligned-columns isn't the default behavior. I can see that it actually used to be the default, and the behavior was changed in #912 to fix #910, but honestly, when data is space-separated, isn't it more likely that it's column-aligned than not? I'd argue that the flag should be reversed, aligned columns should be the default, and opting out from that could be implemented with a flag like --unaligned-columns or something along those lines. WDYT @jntrnr?

@waldyrious Good shout! Has there been a discussion on discord perhaps? I would be open to take this work on in case it's settled.

@waldyrious
Copy link
Contributor Author

Has there been a discussion on discord perhaps?

Not that I'm aware of (I'm not on the Discord server).

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

No branches or pull requests

4 participants