@tertsdiepraam here is the MR I was talking about. And to update ls do I'll wait for the release of the library.

Uh oh, looks like I was just touching the same code 😄 I'll look at this tomorrow.

I'm frankly amazed this works for TopToBottom too. I was just it didn't but it works because we don't reference num_columns in the compute_dimensions for TopToBottom. It's a bit strange. I'm gonna have to poke at this for a bit, because I think there will be some strange edge cases. This is always such hard code to reason about fully.

It calculates the number of possible columns first using the terminal width and widest possible cell content. I thought it's more relevant, since both of the values are given and it can be the "worst" case scenario e.g. all names are the size of widest. Then it tries to increase the number of columns and see if the potential_dimension, which calculates columns widths according to print direction still fits into the terminal width.

I was just it didn't but it works because we don't reference num_columns in the compute_dimensions for TopToBottom.

compute_dimensions calculates the width of the cells according to the direction and it does not use num_columns in TopToBottom because it is the index of the element in cells vector.

If you can come up with some edge cases I didn't think about it would be great, thx

@tertsdiepraam Yeah, sure. If you need to push something, you are more than welcome to

Whoo it finally works. For some reason git had a really hard time pushing to this branch. It's mostly updating some names and comments.

Even tho it fixed the issue I just realized that I drastically increased the number of computations. I'll have a look into this part more later. But if you have any suggestions, just push those

I set a maximum number of columns of self.cells.len(), but otherwise I don't have any great ideas. How are you measuring the performance?

There's actually another algorithm I was thinking of for the LeftToRight direction, which might speed things up again.

Basically we can just start building the first row until we hit the maximum width. This gives us a pretty good maximum number of columns. Then we just decrease the number of columns and calculate the dimensions each time until we find the first one that fits.

For the TopToBottom direction we can then maybe also be a bit smarter by specializing for it. We would need to split the computation for both. Maybe an idea for another PR? Then we'll merge this, then make a PR splitting the two directions and then we can optimize them separately.

How are you measuring the performance?

I'm using hyperfine on uu_ls to see if the change of the crate does not affect performance there. But my take here was to reduce calls to compute_dimensions as much as possible.

Basically we can just start building the first row until we hit the maximum width. This gives us a pretty good maximum number of columns.

I think this one can be tricky with edge cases. We can definitely try this approach to see if it will be more efficient than the one with using the max width.

For the TopToBottom direction we can then maybe also be a bit smarter by specializing for it. We would need to split the computation for both. Maybe an idea for another PR? Then we'll merge this, then make a PR splitting the two directions and then we can optimize them separately.

Actually I was also thinking in this direction, to split the computations for every direction. It will grand more flexibility on improving individual parts, instead of coming up with something common for everything. Plus, there was a request to add RightToLeft direction. If we split these two now, IMHO it will be easier to add support for another the same way.

I have an idea to improve TopToBottom slightly I think. Instead of checking each number of columns we do this:

current_rows = div_ceil(cells, current_cols)
next_rows = current_rows - 1
next_columns = div_ceil(cells, next_rows)

I think (but have to check) that this will skip possibilities that lead to empty columns at the end

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 0.00%. Comparing base (c8db34b) to head (b2d435b).
Report is 1 commits behind head on main.

@@    Coverage Diff     @@
##   main   #48   +/-   ##
==========================
==========================

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.