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

Table column width calculations #107

Open
ghost opened this issue Feb 9, 2020 · 13 comments
Open

Table column width calculations #107

ghost opened this issue Feb 9, 2020 · 13 comments
Assignees

Comments

@ghost
Copy link

ghost commented Feb 9, 2020

I'm currently looking through the code that renders a table of information for display. I haven't gone through it all properly, or gotten my head around it all yet, but it looks to me like the code that prevents a table from being wider than the terminal window has some potential issues that could be improved...
Not sure if these issues exist or are worth trying to fix, but noting my thoughts down here for future just in case.

The current logic seems to go as follows:

  • Get an array of rows and columns
  • Calculate the width of each column in characters (based on the widest content for each)
  • Adjust the allowed width of the widest column so the whole table fits in the terminal
  • Print the header
  • Print the rows, putting the content of the widest column onto multiple lines as necessary

Issues are that there's no check/support for multiline content in the header (I guess we just assume headers are shorter than the content below them), and that if you have a table with lots of wide columns, only the widest is adjusted, which may mean that the table's still too wide for the terminal (or that other columns look disproportionate).

Perhaps and example will help:
Let's work with easy numbers and say that the terminal is 12 wide, and the three columns are 2, 10 and 6 wide respectively. The total table width is 18 (too wide to fit in the terminal (a difference of 6)).
Using the existing logic, this is what'll happen: the widest column (10) will be reduced by the difference (6), so it becomes 4. The columns are now 2, 4, 6 and fit, but it seems weird that the 3rd column gets to stay at its original size.
Imagine adding a 4th column of 3. The widest column is still the 10, but it's now reduced by 9 and so has a width of 1, while all other columns stay the same.

I'm thinking something along these lines might be better logic:

  • Get an array of rows and columns
  • Calculate the width of each column in characters (based on the widest content for each)
  • If the table's too wide for the terminal, get the difference between the two (we'll call this the 'total diff'), then:
    • Divide the terminal width by the number of columns (we'll call this the 'average')
    • For all columns shorter than the average, disperse their difference to the wider columns to create a 'new average'
    • For all columns wider than the new average, try reducing them by the total diff if they'll still be wider than the new average, otherwise reduce them to the new average
  • Print the header, putting any columns wider than their allowed width onto multiple lines
  • Print the rows, putting any columns wider than their allowed width onto multiple lines

Here's an example using the same numbers as the previous example (total width 12):
For the three columns (2, 10, 6): Total diff is 6. Average is 4. First column is shorter than average (by 2), so disperse 2 to the other columns. New average is now 5. Try reducing 2nd column (10) by total diff (6), becomes 4, but that's shorter than new average (5), so make it 5. 3rd column becomes 5 as well. Columns are now 2, 5, 5 (very nice!)
For the four columns (2, 10, 6, 3): Total diff is 9. Average is 3. First column is shorter than average (by 1), so disperse 1 to the other columns (can't have non-integer widths though, so round them down). New average is still 3. Try reducing 2nd column (10) by total diff (9), becomes 1, but that's shorter than new average (3), so make it 3. 3rd column becomes 3 as well. Columns are now 2, 3, 3, 3 (still nice)

New logic could do with some tweaking probably (and likely a better explanation), but at least my thoughts are here for posterity now.

@ghost ghost added the type: enhancement label Feb 9, 2020
@ghost
Copy link
Author

ghost commented Feb 9, 2020

Note to self to check how Drush handles this...

@ghost
Copy link
Author

ghost commented Feb 10, 2020

Can't work out how Drush and other console table formatters do this.

@indigoxela
Copy link
Member

Hey, @BWPanda do you have an actual use-case for this enhancement?

To be honest, the only time I see tables is when I run "b ws" and that one easily fits the console width.

Here's how drush (branch 8.x) seems to do it: https://github.com/drush-ops/drush/blob/03539df0de9974f7b96a06143b54481e081621cf/includes/output.inc#L610

@ghost
Copy link
Author

ghost commented Feb 10, 2020

No, just been looking through the code to see how it works and to try and refactor it. Tables are used, however, everywhere that isn't just plain outputting of a line of text. E.g. messages, help, etc.

@ghost
Copy link
Author

ghost commented Feb 10, 2020

Also, it'd be nice to consolidate the logic for the repeating rows if possible. Currently there's some 'repeating' logic between #L214 and #L248, and similar logic in #L257. Maybe these can become one function that calls itself as necessary?

@yorkshire-pudding
Copy link
Collaborator

Help and messages now use text rather than tables.

@yorkshire-pudding
Copy link
Collaborator

yorkshire-pudding commented Sep 4, 2024

Following a bit of work on #430 and #360 , I'm interested in this. I think the drush approach could be worth trying.

drush_table_column_autowidth()
/**
 * Determine the best fit for column widths.
 *
 * @param $rows
 *   The rows to use for calculations.
 * @param $widths
 *   Manually specified widths of each column (in characters) - these will be
 *   left as is.
 */
function drush_table_column_autowidth($rows, $widths) {
  $auto_widths = $widths;


  // First we determine the distribution of row lengths in each column.
  // This is an array of descending character length keys (i.e. starting at
  // the rightmost character column), with the value indicating the number
  // of rows where that character column is present.
  $col_dist = array();
  foreach ($rows as $rowkey => $row) {
    foreach ($row as $col_id => $cell) {
      if (empty($widths[$col_id])) {
        $length = strlen($cell);
        if ($length == 0) {
          $col_dist[$col_id][0] = 0;
        }
        while ($length > 0) {
          if (!isset($col_dist[$col_id][$length])) {
            $col_dist[$col_id][$length] = 0;
          }
          $col_dist[$col_id][$length]++;
          $length--;
        }
      }
    }
  }
  foreach ($col_dist as $col_id => $count) {
    // Sort the distribution in decending key order.
    krsort($col_dist[$col_id]);
    // Initially we set all columns to their "ideal" longest width
    // - i.e. the width of their longest column.
    $auto_widths[$col_id] = max(array_keys($col_dist[$col_id]));
  }


  // We determine what width we have available to use, and what width the
  // above "ideal" columns take up.
  $available_width = drush_get_context('DRUSH_COLUMNS', 80) - (count($auto_widths) * 2);
  $auto_width_current = array_sum($auto_widths);


  // If we need to reduce a column so that we can fit the space we use this
  // loop to figure out which column will cause the "least wrapping",
  // (relative to the other columns) and reduce the width of that column.
  while ($auto_width_current > $available_width) {
    $count = 0;
    $width = 0;
    foreach ($col_dist as $col_id => $counts) {
      // If we are just starting out, select the first column.
      if ($count == 0 ||
         // OR: if this column would cause less wrapping than the currently
         // selected column, then select it.
         (current($counts) < $count) ||
         // OR: if this column would cause the same amount of wrapping, but is
         // longer, then we choose to wrap the longer column (proportionally
         // less wrapping, and helps avoid triple line wraps).
         (current($counts) == $count && key($counts) > $width)) {
        // Select the column number, and record the count and current width
        // for later comparisons.
        $column = $col_id;
        $count = current($counts);
        $width = key($counts);
      }
    }
    if ($width <= 1) {
      // If we have reached a width of 1 then give up, so wordwrap can still progress.
      break;
    }
    // Reduce the width of the selected column.
    $auto_widths[$column]--;
    // Reduce our overall table width counter.
    $auto_width_current--;
    // Remove the corresponding data from the disctribution, so next time
    // around we use the data for the row to the left.
    unset($col_dist[$column][$width]);
  }
  return $auto_widths;
}

A recent test setting the terminal width to 80 and running with some long project names got this result:

bee projects
| Project                         | Name      | Type   | Status   | Version    |
| admin_bar                       | Administr | module | Enabled  | 1.29.x-dev |
|                                 | ation Bar |        |          |            |
| block                           | Block     | module | Enabled  | 1.29.x-dev |
| book                            | Book      | module | Disabled | 1.29.x-dev |
| ckeditor                        | CKEditor  | module | Disabled | 1.29.x-dev |
|                                 | 4 (Deprec |        |          |            |
|                                 | ated)     |        |          |            |
| ckeditor5                       | CKEditor  | module | Enabled  | 1.29.x-dev |
|                                 | 5         |        |          |            |
| color                           | Color     | module | Enabled  | 1.29.x-dev |
| comment                         | Comment   | module | Enabled  | 1.29.x-dev |
| config                          | Configura | module | Enabled  | 1.29.x-dev |
|                                 | tion Mana |        |          |            |
|                                 | ger       |        |          |            |
| contact                         | Contact   | module | Disabled | 1.29.x-dev |
| contextual                      | Contextua | module | Enabled  | 1.29.x-dev |
|                                 | l Links   |        |          |            |
| dashboard                       | Dashboard | module | Enabled  | 1.29.x-dev |
| date                            | Date      | module | Enabled  | 1.29.x-dev |
| dblog                           | Database  | module | Enabled  | 1.29.x-dev |
|                                 | Logging   |        |          |            |
| email                           | Email     | module | Enabled  | 1.29.x-dev |
| entity                          | Entity    | module | Enabled  | 1.29.x-dev |
| entity_plus                     | Entity Pl | module | Disabled | 1.x-1.0.21 |
|                                 | us        |        |          |            |
| entity_plus_i18n                | Entity Pl | module | Disabled | 1.x-1.0.21 |
|                                 | us Intern |        |          |            |
|                                 | ationaliz |        |          |            |
|                                 | ation Int |        |          |            |
|                                 | egration  |        |          |            |
| entityreference                 | Entity Re | module | Disabled | 1.29.x-dev |
|                                 | ference   |        |          |            |
| field                           | Field     | module | Enabled  | 1.29.x-dev |
| field_sql_storage               | Field SQL | module | Enabled  | 1.29.x-dev |
|                                 |  Storage  |        |          |            |
| field_ui                        | Field UI  | module | Enabled  | 1.29.x-dev |
| file                            | File      | module | Enabled  | 1.29.x-dev |
| filter                          | Filter    | module | Enabled  | 1.29.x-dev |
| image                           | Image     | module | Enabled  | 1.29.x-dev |
| installer                       | Project I | module | Enabled  | 1.29.x-dev |
|                                 | nstaller  |        |          |            |
| language                        | Language  | module | Disabled | 1.29.x-dev |
| layout                          | Layout    | module | Enabled  | 1.29.x-dev |
| link                            | Link      | module | Enabled  | 1.29.x-dev |
| list                            | List      | module | Enabled  | 1.29.x-dev |
| locale                          | Locale    | module | Disabled | 1.29.x-dev |
| menu                            | Menu      | module | Enabled  | 1.29.x-dev |
| node                            | Node      | module | Enabled  | 1.29.x-dev |
| number                          | Number    | module | Enabled  | 1.29.x-dev |
| options                         | Options   | module | Enabled  | 1.29.x-dev |
| path                            | Path      | module | Enabled  | 1.29.x-dev |
| redirect                        | Redirect  | module | Enabled  | 1.29.x-dev |
| search                          | Search    | module | Enabled  | 1.29.x-dev |
| simpletest                      | Testing   | module | Disabled | 1.29.x-dev |
| syslog                          | System Lo | module | Disabled | 1.29.x-dev |
|                                 | gging     |        |          |            |
| system                          | System    | module | Enabled  | 1.29.x-dev |
| taxonomy                        | Taxonomy  | module | Enabled  | 1.29.x-dev |
| telemetry                       | Telemetry | module | Enabled  | 1.29.x-dev |
| text                            | Text      | module | Enabled  | 1.29.x-dev |
| translation                     | Content T | module | Disabled | 1.29.x-dev |
|                                 | ranslatio |        |          |            |
|                                 | n         |        |          |            |
| update                          | Update Ma | module | Enabled  | 1.29.x-dev |
|                                 | nager     |        |          |            |
| user                            | User      | module | Enabled  | 1.29.x-dev |
| views                           | Views     | module | Enabled  | 1.29.x-dev |
| views_aggregator                | Views Agg | module | Disabled | 1.x-1.0.6  |
|                                 | regator P |        |          |            |
|                                 | lus       |        |          |            |
| views_aggregator_more_functions | Views Agg | module | Disabled | 1.x-1.0.6  |
|                                 | regator P |        |          |            |
|                                 | lus More  |        |          |            |
|                                 | Functions |        |          |            |
| views_ui                        | Views UI  | module | Enabled  | 1.29.x-dev |
| bartik                          | Bartik    | theme  | Disabled | 1.29.x-dev |
| basis                           | Basis     | theme  | Enabled  | 1.29.x-dev |
| seven                           | Seven     | theme  | Enabled  | 1.29.x-dev |
| boxton                          | Boxton    | layout | Enabled  | 1.29.x-dev |
| geary                           | Geary     | layout | Enabled  | 1.29.x-dev |
| harris                          | Harris    | layout | Enabled  | 1.29.x-dev |
| moscone                         | Moscone   | layout | Enabled  | 1.29.x-dev |
| moscone_flipped                 | Moscone F | layout | Enabled  | 1.29.x-dev |
|                                 | lipped    |        |          |            |
| rolph                           | Rolph     | layout | Enabled  | 1.29.x-dev |
| simmons                         | Simmons   | layout | Enabled  | 1.29.x-dev |
| sutro                           | Sutro     | layout | Enabled  | 1.29.x-dev |
| taylor                          | Taylor    | layout | Enabled  | 1.29.x-dev |
| taylor_flipped                  | Taylor Fl | layout | Enabled  | 1.29.x-dev |
|                                 | ipped     |        |          |            |

@yorkshire-pudding
Copy link
Collaborator

The auto column width seems to work well with a bit of tweaking but other bits of the table writing will also need doing as it currently only wraps the text from the longest column.

@yorkshire-pudding
Copy link
Collaborator

yorkshire-pudding commented Nov 19, 2024

I've been thinking about this again and wondering whether some of the implicit assumptions about table width are still valid.
The current code and the discussion so far in this ticket assumes that we should wrap text so the table fits in to the current terminal width. Is that a valid assumption?

  • The logic can get pretty complex when working out how to do it and improve it
  • I encountered a situation where the table rendering crashed:
    • Lando was assuming the COLUMNS was 80 (separate issue with Lando)
    • I was using bee projects on an install that had lots of modules I maintain and many of the version strings were long in addition to several long project names (both machine and human readable). The current code attempted to reduce the width below zero, which caused a problem.
  • Reading More consistent table formatting #108 the comparison is made with MySQL query results which doesn't wrap long text. Many tables look terrible, but by expanding the terminal emulator window you can view it.
  • Most/All(?) people will be using bee with a terminal emulator with wide enough screens that they can expand it if necessary.
  • If Add --csv option to relevant commands #185 is implemented and we add the ability to output in both csv and json does the need to always fit the current terminal width still apply?

I've been experimenting with a simpler approach and the only one I had to expand the terminal window for (and it still fitted onto a single 21" screen @ 1920 x1080 ) was bee log where some long error messages existed.

I'm currently looking at doing this ticket, #108 and #185 as one refactor of the tabular output.

@indigoxela - particularly interested in your views as you've participated in this discussion, but open to any other input.

If I don't get any feedback in the next week or so, I'll just go ahead.

@indigoxela
Copy link
Member

@indigoxela - particularly interested in your views as you've participated in this discussion

TBH, I scratch my head to get the problem in the first place. 😉

When I use 'bee ws', the table fits nicely into the console window. Regardless of how wide or narrow the console window currently is and regardless how verbose the dblog messages are. I'm using just a Linux terminal - on native Linux.
What am I missing?

@yorkshire-pudding
Copy link
Collaborator

The current code wraps the table text and reduces the largest column's width. I'm proposing to ditch the text wrapping and just calculate column widths as the max for each column.
As an example, if the first sentence was in a cell:

This is what it might look like now

The current code wraps the table text and
reduces the largest column's width. 

This is what is would look like

The current code wraps the table text and reduces the largest column's width. 

@izmeez
Copy link

izmeez commented Nov 24, 2024

I'm also having difficulty understanding the problem that is looking for a solution. Bee seems to be displaying fine for me. Granted the terminal is not restricted to 80 cols. But whether on Linux or Windows the display looks fine apart from occasional words being chopped.

@yorkshire-pudding
Copy link
Collaborator

I don't expect you to be able to reproduce but I do take notice of edge conditions when I find them and it is feasible that others could encounter issues.

Also the column calculations, as outlined by BWPanda, are not optimal and could result in the column with the largest content being the smallest as it is reduced.

This is what happens when the terminal is set to 80

COLUMNS = 80
 ℹ  'Debug' mode enabled.
 🕵  [bee_get_column_widths:363] Column widths:
Array
(
    [0] => 29
    [1] => 44
    [2] => 6
    [3] => 8
    [4] => 47
)

 🕵  [bee_get_column_widths:383] The terminal width is: 80
| Project                       | Name                                         | Type   | Status   | Version                 |
| action_example                | Action example                               | module | Disabled |                         |

 ⚠️  Warning: Cannot modify header information - headers already sent by (output started at /app/bee/includes/render.inc:175)
in backdrop_send_headers() (line 1646 of /app/docroot/core/includes/bootstrap.inc).

Error: Xdebug has detected a possible infinite loop, and aborted your script with a stack depth of '512' frames in bee_format_text() (line 216 of /app/bee/includes/render.inc).

Hopefully you can see

Hopefully you can see what is happening here:

  • largest column gets reduced to zero
  • other columns still more than terminal width
COLUMNS = 140
 lando bee projects -d
 ℹ  'Debug' mode enabled.
 🕵  [bee_get_column_widths:363] Column widths:
Array
(
    [0] => 29
    [1] => 44
    [2] => 6
    [3] => 8
    [4] => 47
)

 🕵  [bee_get_column_widths:383] The terminal width is: 140

This is the longest column so gets wrapped
image

But there is another long column that if we were to wrap too, things might fit on 80.
image

And another long column:
image

As writing a function that will decide how to wrap multiple long columns could be very complex, I am proposing not to reduce any widths or wrap the text in any columns - to simply display columns at the width they need and let people expand their terminal window if necessary.

I don't mind if you have no opinion.

@yorkshire-pudding yorkshire-pudding self-assigned this Nov 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants