tabled icon indicating copy to clipboard operation
tabled copied to clipboard

Variable header length for Enum according to variant

Open y-young opened this issue 2 years ago • 8 comments

Hi, I tried this library recently, overall it's great, but there's still a point I'm concerned about: Is it possible to adjust header length of enums according to the variant?

Take the one in readme as an example:

#[derive(Tabled)]
enum Vehicle {
    #[tabled(inline("Auto::"))]
    Auto {
        model: &'static str,
        engine: &'static str,
    },
    #[tabled(inline)]
    Bikecycle(
        #[tabled(rename = "name")] &'static str,
        #[tabled(inline)] Bike,
    ),
}

#[derive(Tabled)]
struct Bike {
    brand: &'static str,
    price: f32,
}

fn main() {
    let v = Vehicle::Auto {
        model: "A",
        engine: "B",
    };
    println!("{}", Table::new(vec![v]));
}

This produces:

+-------------+--------------+------+-------+-------+
| Auto::model | Auto::engine | name | brand | price |
+-------------+--------------+------+-------+-------+
|      A      |      B       |      |       |       |
+-------------+--------------+------+-------+-------+

I wonder if the empty columns could be omitted automatically, like this:

+-------------+--------------+
| Auto::model | Auto::engine |
+-------------+--------------+
|      A      |      B       |
+-------------+--------------+

And this for Bikecycle variant:

+------+-------+-------+
| name | brand | price |
+------+-------+-------+
| bike |   A   |  100  |
+------+-------+-------+

I'm thinking maybe it could work if Tabled::LENGTH is a method fn length(&self) -> usize.

Thank you.

y-young avatar May 03 '22 07:05 y-young

Hi @y-young

Thank you for opening this issue.

So do you essentially mean that if a vector has only 1 variant we shall not render columns from other variants right?

I think it may be good, but I wonder the way to do that 'automatically'. And honestly I questioning myself even if we could do it that way. Is this the right behavior?

Because if you know that there will be only Bikes or Autos why not to use a general struct? Or cast a Vec<Vehicle> to the necessary subtype like Vec<Auto>.

Sort of like this

impl Vehicle {
    fn as_bike(&self) -> Option<(&str, &Bike)> {
        match self {
            Vehicle::Bikecycle(s, b) => Some((s, b)),
            _ => None,
        }
    }
}

fn main() {
    let data = vec![
        Vehicle::Bikecycle(
            "123,",
            Bike {
                brand: "BMW",
                price: 1.0,
            },
        ),
        Vehicle::Bikecycle(
            "23,",
            Bike {
                brand: "Honda",
                price: 1.0,
            },
        ),
    ];

    let table = data.iter().flat_map(|v| v.as_bike()).table();

    println!("{}", table);
}
+------+-------+-------+
| &str | brand | price |
+------+-------+-------+
| 123, |  BMW  |   1   |
+------+-------+-------+
| 23,  | Honda |   1   |
+------+-------+-------+

About this casting.

Just a thought that potentially we could generate this casts preserving all the #[tabled(...)] information.

So the usage would look like this.

Vehicle::table::Auto::from(v)

But it's not 'automatic' in any way because you chose the way you want to crate the table.

The next approach is an 'automatic'.

What we could do is create a new primitive like Disable::empty which would delete empty rows and columns. (It could be done manually even now via indexing so in your case it would be Disable::Column(3..))

Would be used as follows.

Table::new(vec![v]).with(Disable::empty());

It's an interesting idea overall.

But doing it the way you described would require to do some changes in Tabled trait.

I'm thinking maybe it could work if Tabled::LENGTH is a method fn length(&self) -> usize.

I don't think it will help because the logic you propose must be determined where we get a data vector.

Let me know your thoughts.

zhiburt avatar May 03 '22 13:05 zhiburt

Because if you know that there will be only Bikes or Autos why not to use a general struct? Or cast a Vec<Vehicle> to the necessary subtype like Vec<Auto>.

That'll work for a single enum, but if it's a field inside another struct like this:

pub struct Object {
    pub name: String,
    pub resource: Resource,
}

And we would like to print it with name, then I guess it would need some concatenation work.

About this casting.

Just a thought that potentially we could generate this casts preserving all the #[tabled(...)] information.

So the usage would look like this.

Vehicle::table::Auto::from(v)

But it's not 'automatic' in any way because you chose the way you want to crate the table.

Yes, you're right. It would be better if it can automatically detect the variant type.

The next approach is an 'automatic'.

What we could do is create a new primitive like Disable::empty which would delete empty rows and columns. (It could be done manually even now via indexing so in your case it would be Disable::Column(3..))

That would be convenient. But what if the rows have different numbers of empty columns? In that case the table may be broken.

I'm thinking maybe it could work if Tabled::LENGTH is a method fn length(&self) -> usize.

I don't think it will help because the logic you propose must be determined where we get a data vector.

The intuition is to pass the computation of LENGTH down to the variant. We derive Tabled on the inner value type (ResourceA), when implementing trait for enum, we call the corresponding method of the inner type:

impl Tabled for Resource {
    fn length(&self) -> usize {
        match self {
            Resource::A(_) => ResourceA::length(),
            _ => 1,
        }
    }
}

Same for fields and headers, in this way we can construct the table according to the variant.

Apparently this is a primitive approach and it's not automatic enough. I tried to modify the code but couldn't get it done since I'm new to Rust. I wonder if this makes sense to you.

Thanks for replying.

y-young avatar May 03 '22 14:05 y-young

Apparently this is a primitive approach and it's not automatic enough. I tried to modify the code but couldn't get it done since I'm new to Rust. I wonder if this makes sense to you.

Well neither I am an expert :) that's why you couldn't get it done :disappointed_relieved:

That would be convenient. But what if the rows have different numbers of empty columns? In that case the table may be broken.

We would not touched these rows. We would delete only rows and columns which are empty, completely.

And actually because of this this will not work :( Because the column titles are note empty in your case.

The intuition is to pass the computation of LENGTH down to the variant. We derive Tabled on the inner value type (ResourceA), when implementing trait for enum, we call the corresponding method of the inner type:

According to your example.

But if there will be a vec![Resource::B] it wont be printed?

And what would you do then in cases like this.

impl Tabled for Resource {
    fn length(&self) -> usize {
        match self {
            Resource::A => ResourceA::length(), // 3
            Resource::B => ResourceB::length(), // 5
        }
    }
}

// they have a different length value.
let data = vec![Resource::A, Resource::B].

Just to clarify. It's your expectation correct?

fn main() {
    let auto = Vehicle::Auto { model: "A", engine: "B" };
    let bike = Vehicle::Bikecycle("B", Bike { model: "A", price: 2.12} );
   
// +-------------+--------------+
// | Auto::model | Auto::engine |
// +-------------+--------------+
// |      A      |      B       |
// +-------------+--------------+
    println!("{}", Table::new(vec![auto]));


// +------+-------+-------+
// | name | brand | price |
// +------+-------+-------+
// | bike |   A   |  100  |
// +------+-------+-------+
    println!("{}", Table::new(vec![bike]));


// +-------------+--------------+------+-------+-------+
// | Auto::model | Auto::engine | name | brand | price |
// +-------------+--------------+------+-------+-------+
// |      A      |      B       |      |       |       |
// +-------------+--------------+------+-------+-------+
// |             |              |  A   |  B    |       |
// +-------------+--------------+------+-------+-------+
    println!("{}", Table::new(vec![auto, bike]));
}

zhiburt avatar May 03 '22 15:05 zhiburt

And actually because of this this will not work :( Because the column titles are note empty in your case.

But it could be done in Builder.

The usage would look like the following.

Table::builder(vec![v]).clean().build()

(Just in case) see https://github.com/zhiburt/tabled/blob/d4a2077419a1f62acdec2ecd57b1baff2565a68f/src/builder.rs#L213

zhiburt avatar May 03 '22 15:05 zhiburt

But if there will be a vec![Resource::B] it wont be printed?

I just omitted it, never mind.

Just to clarify. It's your expectation correct?

fn main() {
    let auto = Vehicle::Auto { model: "A", engine: "B" };
    let bike = Vehicle::Bikecycle("B", Bike { model: "A", price: 2.12} );
   
// +-------------+--------------+
// | Auto::model | Auto::engine |
// +-------------+--------------+
// |      A      |      B       |
// +-------------+--------------+
    println!("{}", Table::new(vec![auto]));


// +------+-------+-------+
// | name | brand | price |
// +------+-------+-------+
// | bike |   A   |  100  |
// +------+-------+-------+
    println!("{}", Table::new(vec![bike]));

Exactly.

// +-------------+--------------+------+-------+-------+
// | Auto::model | Auto::engine | name | brand | price |
// +-------------+--------------+------+-------+-------+
// |      A      |      B       |      |       |       |
// +-------------+--------------+------+-------+-------+
// |             |              |  A   |  B    |       |
// +-------------+--------------+------+-------+-------+
    println!("{}", Table::new(vec![auto, bike]));
}

I haven't thought about this before, but this makes sense to me.

y-young avatar May 03 '22 15:05 y-young

Good I'll try to think about this idea.

But I'd love to know what do you think about this

The usage would look like the following.

Table::builder(vec![v]).clean().build()

As I said the logic here is

  • when the column has all cells empty - It will be removed.
  • when the row has all cells empty - It will be removed.

Would it be OK?

zhiburt avatar May 03 '22 16:05 zhiburt

Good I'll try to think about this idea.

Thanks a lot.

But I'd love to know what do you think about this

The usage would look like the following.

Table::builder(vec![v]).clean().build()

As I said the logic here is

  • when the column has all cells empty - It will be removed.
  • when the row has all cells empty - It will be removed.

Would it be OK?

Looks good to me, although it doesn't match my need exactly, I'm sure someone will be happy to have this feature.

y-young avatar May 03 '22 16:05 y-young

Just a note:

I've merged Builder::clean method.

See https://github.com/zhiburt/tabled/blob/ddc81adea60e33bd06b6cdeb0a3911386a8e6194/src/builder.rs#L63-L105

If you want you can experiment with this by using from git.

tabled = { git = "https://github.com/zhiburt/tabled/", branch = "master" }

zhiburt avatar May 04 '22 17:05 zhiburt