I've noticed that most of the stdout output of Drush does not wrap cleanly at 80 characters, the standard default width for a terminal. I would like to see Drush move in the direction of having a clean 80 character output for the commands where it is reasonable to do so. I realize commands like "statusmodules" needs the width, however there are places that really don't need the width and could be cleaned up to work on the default standard.

For instance the output of "help" looks really bad at 80 characters at places:

  cache clear           Clear all caches.                                                            

  watchdog show         Shows recent watchdog log messages. Optionally filter fo
r a specific type.   
  watchdog delete       Delete all messages or only those of a specified type.                       

  sync                  Rsync the Drupal tree to/from another server using ssh. 

Since this is invoked when no Drush command is supplied, and one of the first things that most people will run, it seems really important to make it conform to an accepted standard.

I'm happy to provide some patches that do this, but am not sure where to start.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

adrian’s picture

I used to have it wrapped at 80 (at least the log messages) , but the messages never displayed cleanly, because they would break up in the weirdest places.

What I would like, is to be able to figure out how many chars I can print to, and change it on the fly based on the terminal.

Owen Barton’s picture

Yeah - I think there is definitely space for some improved wrapping helpers. Another nice feature would be able to wrap with an indent (ideally with a one indent parameter for the first line and another for all subsequent lines, so you can wrap option/command descriptions neatly).

Figuring out the terminal size may be hard to do 100% reliably on all systems - a default of 80 is probably a safe start though. Some terminals allegedly set a COLUMNS environment variable, although mine seems to not bother. We could check if the php ncurses is available (which seems pretty common), and if so use ncurses-getmaxyx(). It looks like there is an equivalent function in the newt library newt_get_screen_size(), although I haven't seen that installed anywhere by default. There is probably some common command line thing that tells us the dimensions (although I couldn't immediately find one) we could try exec()ing too, although that might introduce some latency.

At the same time, we might want to have a way to disable wrapping, since it can be a pain if you are not running in an interactive way (e.g. if you want another script to cut/grep through drush output).

gnat’s picture

My machine definitely sets a $COLUMNS variable. On my Debian laptop I can do this:

0 nat@pigtown:~$ echo $COLUMNS
80

Then I change my terminal from 80, to full screen width, and run it again:

0 nat@pigtown:~$ echo $COLUMNS
166

I don't know if this works in all POSIX systems, but it works in the bash shell, as well as ksh and tsh. It also worked in a genuine console window (not through a terminal app).

Generally though, I tend to think that setting a hard standard of 80 characters is okay in situations where the content will bear it. This is likely a limited subset, like "help" output, which probably won't be used interactively, and will look fine wrapped at 80 characters even in a larger terminal window.

The point about interactive use is really good one. I would indeed like to see errors not broken up when I'm grepping them, so a no break option might be really helpful.

adrian’s picture

I'd like to properly right-align the [error] and other message tags during display.

the issue i had was the $COLUMNS environment variable isn't in the php $_ENV or $_SERVER, but moshe pointed out we can use the backtick to get it :

php -r "print `echo $COLUMNS`;"

We'll fix this soon enough.

moshe weitzman’s picture

We still have to figure out how to wrap text within a table cell (see command listing and help for a given command). Interested parties should look at drush_print_table().

moshe weitzman’s picture

Priority: Minor » Critical

I would really like to see this before final 2.0 release.

moshe weitzman’s picture

I tried a few bad scripts on the interwebs for turning tabular data into an ascii table. the best one is a PEAR class called console_table. see http://pear.php.net/manual/en/package.console.console-table.php. But you have to wordwrap() long texts yourself which is an inconvenience.

Again, i think drush help is the most important use case for this.

adrian’s picture

mmmmm
the echo columns works fine from the -r flag, but not when you are in a php script (or with the interactive -a flag).

$x = `echo \$HOME`;
print($x);
/Users/adrian
$x = `echo \$COLUMNS`;
print($x);

Notice the last line is blank.

adrian’s picture

so. the issue appears to be (at least in my environment) , that the COLUMNS value is not exported, thus it does not appear in any of the other shells.

Moya:~ adrian$ sh -l -c 'echo $COLUMNS';

Moya:~ adrian$ export COLUMNS=$COLUMNS;
Moya:~ adrian$ sh -l -c 'echo $COLUMNS';
175
adrian’s picture

I tried. even in a standard bash shell script, the $columns variable is always unavailable, unless it has been exported.

adrian’s picture

I suppose the only compromise we can make, is to use $COLUMNS when it has been exported, and NCurses if available, and otherwise default to 80 columns.

and document that in the readme

adrian’s picture

i am skipping ncurses support entirely.

it's got too many drawbacks, and is marked as experimental : http://us2.php.net/manual/en/function.ncurses-getmaxyx.php

adrian’s picture

i committed the loading of COLUMNS and added the documentation for it.

but i can't figure out how to make drush_print_table work correctly. it's super tricky.

adrian’s picture

Perhaps we should instead simulate dl tags

ie:

dsfasdfsdfsdfasfd
sadfasfasdfadsfasdfasdfasdfasdfasfadff

sadfasdfdfdasfaf
sdfasdfasdfasdfasdfsafasfdsfadsfadsfasdf

This can be done _far_ more cleanly and simply than trying to build tables.

adrian’s picture

i found a better mechanism than columns, if it doesn't exist.

>> system('stty size');
46 148

dunno if it's available on windows.

adrian’s picture

Status: Active » Needs review
FileSize
6.7 KB
26.03 KB

stty is not available on windows, and i don't have windows anywhere to test on, so frankly i don't care.

I don't even know if $COLUMNS is available on windows.

--
I wrote this patch using the Console_Table class in pear. (http://pear.php.net/package/Console_Table/docs/latest/Console_Table/Cons...)
It's a single include file with no weird Pearisms. so i think we should just fork it.

Move table.inc to the drush/includes folder.

I also changed how $headers work, and i can't get table indent to work correctly.

This is the last time i'm touching this issue, someone else can take over from here.

example output : http://pastebin.com/m2e7cf970

moshe weitzman’s picture

FileSize
4.3 KB

I worked on a patch independant of adrian. Here is mine. Lets compare and take the best from each. Mine uses the pear console_table if you have it installed. To install it, do sudo pecl install console_table.

moshe weitzman’s picture

FileSize
5.12 KB

Minor tweaks over the last one ... You can test these with the commands `drush help`, drush help dl, and drush watchdog show.

moshe weitzman’s picture

lets see what folks think of the 2 patches. feedback wanted. neither of us is thrilled with our patches.

anarcat’s picture

I too am not thrilled by either. How about the idea of not outputting tables but "definition lists"? wouldn't that resolve the problem altogether?

So instead of doing:

drush foo bar     do foo bar stuff on thing blah
                  bleh quux thing.

You would do:

drush foo bar
     do foo bar stuff on thing blah
     bleh quux thing.

Easier to wordwrap, cleaner...

moshe weitzman’s picture

and doubles the number of lines. thats not acceptable for a CLI app IMO.

Owen Barton’s picture

I'll try and review &| merge these when I get a chance - I think some kind of solution here will make our lives much easier. I think we will still need some manual tinkering here and there to make sure column widths stay sane, but wrapping text by hand is soooo 1970s...

Owen Barton’s picture

Assigned: Unassigned » Owen Barton
Status: Needs review » Needs work
adrian’s picture

personally. i feel this shouldn't hold up release.

Owen Barton’s picture

Status: Needs work » Needs review
FileSize
40.2 KB

Here is a patch that I think makes some fairly major improvements in default table layouts. It is basically a very simplified version of what browsers do to determine table width - it figures out an approximately optimal fit for any column widths not specified by looking at the distribution of row lengths and balancing the columns such as to limit the number of characters wrapped across the table.

I am not sure if this should go in to a stable release - it still needs a little more polishing and testing on some of the more complex tables. Feels like a good start to me though.

Owen Barton’s picture

FileSize
40.2 KB

Minor fix for table headers.

adrian’s picture

Status: Needs review » Reviewed & tested by the community

Patch works well.

adrian’s picture

Status: Reviewed & tested by the community » Needs work

Ok. on closer inspection :

The patch to command.inc looks like gunk from http://drupal.org/node/418208

Index: includes/command.inc
===================================================================
RCS file: /cvs/drupal-contrib/contributions/modules/drush/includes/command.inc,v
retrieving revision 1.31
diff -u -r1.31 command.inc
--- includes/command.inc	11 May 2009 14:43:34 -0000	1.31
+++ includes/command.inc	25 May 2009 08:00:08 -0000
@@ -413,7 +413,15 @@
         $files = drush_scan_directory($path, '\.drush\.inc$');
         foreach ($files as $filename => $info) {
           require_once($filename);
-          $list[basename($filename, '.drush.inc')] = $filename;
+          $base = basename($filename, '.drush.inc');
+          $info = array('status' => 1);
+          $func = $base . '_drush_info';
+          if (function_exists($func)) {
+            $info = $func();
+          }
+          if ($info['status']) {
+            $list[$base] = $filename;
+          }
         }
       }
     }

That shouldn't be committed.

Owen Barton’s picture

Status: Needs work » Needs review
FileSize
39.31 KB

Patch without sneaky impostor chunk.

moshe weitzman’s picture

Status: Needs review » Needs work

This works well for me too. However, it spews Notices. May I suggest that developers change their drush alias to something like below:

alias drush='php -d error_reporting=E_ALL ~/contributions/modules/drush/drush.php';

That should show the notices that need fumigating.

We should also chat about the new include file. Lets meet in #aegir or on Jabber.

Owen Barton’s picture

Status: Needs work » Needs review

Good idea - I'll fix those notices and post a revised patch.

Feel free to ping me on #aegir and we can chat about the .inc - my sense was that it is better to include it than assume folks PEAR paths being set up sensibly (experience having shown that pretty much every assumption about the running environment is false on at least one system...), and also that we don't have to worry about it picking up ancient versions. The library seems pretty established, and we should be able to keep up with upgrades without trouble. I am actually wondering if I should contribute my column auto-sizing patch upstream, since it seems like it should be generally useful (and would be less code for us to maintain).

Owen Barton’s picture

FileSize
12.32 KB

OK - attached patch fixes the warnings and a couple of minor bugs.

After thinking about the options I think an automated download is the cleanest choice for now. This patch doesn't include table.inc, but instead adds a simple check and automatic (fopen wrapper) download from http://cvs.php.net/viewvc.cgi/pear/Console_Table/Table.php?revision=1.28... if the file is not found. If it is still not found it outputs instructions on manually installing it. If we find many people don't have fopen Url enabled on their cli php we could also try a fallback wget/curl download.

moshe weitzman’s picture

Great idea. I do think we are going to save a lot of support headaches for users and ourselves by putting in the wget/curl fallback. That feature is commonly disabled on shared hosting.

Owen Barton’s picture

FileSize
13.88 KB

Added both wget and curl fallbacks

moshe weitzman’s picture

Status: Needs review » Reviewed & tested by the community

I tested this and am satisfied. Owen is welcome to commit this when he's ready. We talked about creating a drush 'workspace' for holding table.php and other semi-permanent downloads like imagined in sql sync command. That may or may not get implemented in this issue.

Owen Barton’s picture

Status: Reviewed & tested by the community » Fixed

Committed - thanks all :)

Adrian/Moshe - remember to add table.inc to an includes/.cvsignore file, so we don't accidentally commit this library. If you like feel free to commit an includes/.cvsignore to this effect (probably not one in the root - need to keep that one separate, so we can ignore IDE junk).

naught101’s picture

With the latest CVS, everything is wrapped to 80 chars for me, although my $COLUMNS is 143. It makes the help output have maybe 80% more lines than it needs. Was that intentional? It's definitely not as nice.

naught101’s picture

Also, the tables layout makes post-editing the output of commands like "drush statusmodules" much harder, as this:
Per-Entity Logs (workflow_ng_log) Disabled Provides per-user and per-node logs which can be written to ...
becomes this:

Per-Entity Logs       Disabled  Provides per-user and per-node logs which can
 (workflow_ng_log)               be written to  ...

But only for modules with long names - some modules have the two names one one line. With really long module names, some end up on up to 4 lines:

 Bonus: Sparkline      Disabled  View type that outputs a each field as a
 view                            sparkline graph, wi ...
 (views_bonus_sparkli
 ne)

For example, I might want to run "drush statusmodules" one multiple sites, grep for the module names (in brackets), and then uniq-d to count which modules are installed on how many sites. This is now much harder.

moshe weitzman’s picture

I can't reproduce that wrapping. Working nicely for me. I suspect drush is not learning about $COLUMNS. Please use drush script instead of drush.php (see latest version).

Sheldon Rampton’s picture

I just encountered an issue with this when I tried installing Drush All-Versions-2.0. I ran drush.php dl zen and got the following error message:

Drush needs a copy of the PEAR Console_Table library saved as Drush  [error]
includes/table.inc. Drush attempted to download this automatically,
but failed. To continue you will need to download the 1.1.3 package
from http://pear.php.net/package/Console_Table, extract, and move the
file Table.php to includes/table.inc.
Drush could not execute.

After I followed these instructions and manually moved Table.php to includes/table.inc, Drush seemed to work fine when I used it to download zen.

FYI, I did this on the same server where I previously reported an issue titled "Can't download projects with dl":
http://drupal.org/node/458772

I can confirm that the error reported in that issue no longer occurs, so that issue at least seems to be resolved. I see that it has already been closed. Well done, gents!

naught101’s picture

Same here, everything's working now though

Senpai’s picture

Status: Fixed » Active

Ok, ok, I was having the same problem as #40 and #41, but since Drush couldn't auto-download the needed file into MAMP and was giving me an error, I downloaded it myself...

and put it into /www/example.com/includes/table.inc. That's exactly what the instructions say to do. What they *mean*, however, is to place the file into DRUSH's dir at /opt/local/sbin/drush/includes/table.inc, or whatever your local path to Drush is.

Doh! I'm glad I found this thread, or I never would have gotten the hint that the needed file was supposed to be inside of Drush rather than Drupal. Want a patch to the help text?

BTW, my Drush 2 is working perfectly now. :)

moshe weitzman’s picture

of course yes. patches welcome. thanks.

Owen Barton’s picture

@Senpai - what was the error Drush was giving when it tried to auto-download table.inc?

hutch’s picture

FileSize
2.11 KB

I also had problems getting table.inc to download automatically, so I had a look at function _drush_bootstrap_drush_validate() in includes/environment.inc
I realised that there was a bug in the code that would cause wget or curl to place table.inc in drupal/includes not drush/includes. urk ;-(

The attached patch fixes that, and attempts to clarify the error message.

HTH

Owen Barton’s picture

Status: Active » Fixed

Well spotted, I think that explains the issues here.

Committed - thanks!

Senpai’s picture

Oh, nice! Much better to fix the 0.1% problem than try to explain it away with more text. Thanks, Hutch, and now i won't submit a patch.

Senpai’s picture

Priority: Critical » Minor
Status: Fixed » Needs review
FileSize
1.45 KB

I came back to this issue cause something was bothering me about it, and after manually triggering the error, the text was actually too verbose. Its even more difficult than before, so here's an attempt to make it more easily understood:

hutch’s picture

This patch applies OK and gives a better error message
Tested on environment.inc v 1.37 in HEAD

Owen Barton’s picture

Status: Needs review » Fixed

Committed - thanks!

naught101’s picture

Status: Fixed » Active
Issue tags: +commandline

just when you thought it was all wrapped up (sorry :P)

It's actually useful to have no wrapping sometimes, for example, if you're trying to parse the output of drush with another script.

A good way to deal with this might be to have no wrapping at all if running drush.php, and only wrap when running the "drush" script. the current default of 80 is pretty much useless for commands with lots of output, like "drush statusmodules".

The other option would be to include a --nowrap switch, for turning off wrapping when needed.

intuited’s picture

FileSize
1.21 KB

This is a patch to make use of stty to set the number of columns, aka the display width. It will only attempt to do so if the COLUMNS environment variable is not set, and should fail cleanly if there's no command named 'stty' available on the system (even under Windows).

The patch is against the 2.0 source code but can be applied against the current HEAD code.

I got sick of seeing cramped output, and figured there was potentially some very good reason why COLUMNS is not exported by default.

I haven't tested this at all under Windows (I don't usually run it), but according to Google the 2>&1 redirection of standard error should work okay under everything NT, XP, and later, including (some versions of?) CE. This means that although it won't get a column width under Windows, it also won't output an error message or otherwise interfere with the copacetic operation of drush, and will degrade gracefully to 80 column output.

It's expecting stty to output its info as per comment #15 above, which is the way it works when running the GNU coreutils implementation of stty with or without the POSIXLY_CORRECT environment variable defined and exported, so presumably everywhere that such notions exist.

If there is instead a different command called "stty" in the executable path that, when called with an argument of 'size', returns 0 exit status but produces different output, or does something unpredictable, then mayhem may ensue.
intuited’s picture

Also @naught101: to disable wrapping you could do something like

$ env COLUMNS=999 drush statusmodules

Though a --nowrap switch would definitely be more convenient.

Owen Barton’s picture

Note that we already do something very similar to this with the "drush" wrapper - we do "export COLUMNS=$(tput cols)" (tput cols gives a raw number, so is much easier to manage that stty). Is this not working for you, or is there some reason you can't use this wrapper?

intuited’s picture

I'm not running the drush bash script because I built a custom bash script before the bash wrapper script was added to drush. It does work fine that way for me. I actually didn't realize that this was being exported from the drush bash script, and for some reason it didn't occur to me to just add the export to my script.

It does seem to be a pretty good wheel, as it were..... There isn't really much advantage to doing this from within the php code, except to support people who don't use bash but whose systems do have stty available.

It seems like there would be a very slight portability advantage in using stty, since tput is only installed with curses, which in theory some people might not be using. That does seem extremely unlikely.

intuited’s picture

When I'm running drush as a shell command under vim, eg using the Ex command
:r !drush statusmodules
I'm getting the results in 80 columns. This is the only situation where I've had this problem since starting to use the drush.bash wrapper. It works this way on the three different ubuntu systems that I tried it on. There may be a vim option that fixes this, but then there may be other situations in which tput doesn't get the terminal width.

Some investigation reveals that, when running under vim, "tput cols" will return 80 where "stty size" will return the actual terminal width. so running this ex command
:r !stty size; tput cols
will give this output:

39 133
80

Andrey Zakharov’s picture

definitely broken in 2.1
for tput cols = 157

~/www/htdocs/vaultsoft.ru]$ drush  sm       
 Имя                Состояни  Описание                                  
                    е                                                   
 Actions (actions)  Отключен  Provides an API for modules to trigger    
                    о         and respond to Dr ...                     
 Activeedit         Отключен  Provides AJAX-based in place editing.     
 (activeedit)       о                                                   
 Activemenu         Отключен  Adds AJAX-based tree menu to navigation   
 (activemenu)       о         menu.                                     
 Active Search      Отключен  Adds AJAX to standard Drupal search.      
 (activesearch)     о                                                   
 Administration     Включено  Provides a dropdown menu to most          
 menu (admin_menu)            administrative tasks an ...               
 Advanced Taxonomy  Отключен  A modification of Taxonomy Menu that      
 Menu               о         creates the menu fr ...                   
 (adv_taxonomy_men                                                      
 u)                                                                     
 Aggregator         Включено  Aggregates syndicated content (RSS, RDF,  
 (aggregator)                 and Atom feeds).                          
 Ajax Views         Отключен  Implementation of a dynamic view block    
 (ajax_views)       о         that can paginate ...                     
 AJAX submit        Отключен  Makes designated forms submit via AJAX.   
 (ajaxsubmit)       о                                                   

but with commented out cycle in includes/drush.inc

- while ($auto_width_current > $available_width) {
+/*while ($auto_width_current > $available_width) {

looks like:

~/www/htdocs/vaultsoft.ru]$ drush  sm
 Имя                                                        Состояние  Описание                                                     
 Actions (actions)                                          Отключено  Provides an API for modules to trigger and respond to Dr ... 
 Activeedit (activeedit)                                    Отключено  Provides AJAX-based in place editing.                        
 Activemenu (activemenu)                                    Отключено  Adds AJAX-based tree menu to navigation menu.                
 Active Search (activesearch)                               Отключено  Adds AJAX to standard Drupal search.                         
 Administration menu (admin_menu)                           Включено   Provides a dropdown menu to most administrative tasks an ... 
 Advanced Taxonomy Menu (adv_taxonomy_menu)                 Отключено  A modification of Taxonomy Menu that creates the menu fr ... 
 Aggregator (aggregator)                                    Включено   Aggregates syndicated content (RSS, RDF, and Atom feeds).    
 Ajax Views (ajax_views)                                    Отключено  Implementation of a dynamic view block that can paginate ... 
 AJAX submit (ajaxsubmit)                                   Отключено  Makes designated forms submit via AJAX.                      
 Archive (archive)                                          Отключено  Allows visitors to view content filtered by date.            
 Autolocale (autolocale)                                    Отключено  Automatically imports interface translations.                
 Block (block)                                              Включено   Controls the boxes that are displayed around the main co ... 
 Blog (blog)                                                Включено   Enables keeping easily and regularly updated user web pa ... 
 Blog API (blogapi)                                         Включено   Allows users to post content using applications that sup ... 
 Book (book)                                                Включено   Allows users to structure site pages in a hierarchy or o ... 
 Boost (boost)                                              Включено   Caches text (html, ajax, xml) outputted by Drupal as sta ... 
Owen Barton’s picture

Status: Active » Fixed

On consideration committed #52, as it is only run if tput (or a manual export) fails, and I could reproduce the tput issue when running from other programs (e.g. Eclipse). I also committed a separate tweak to the drush shell script to suppress the tput error message ("tput: No value for $TERM and no -T specified) in these cases.

No idea what is going on in #57 - please open a separate issue with more info if this is an issue with HEAD.

Status: Fixed » Closed (fixed)
Issue tags: -commandline

Automatically closed -- issue fixed for 2 weeks with no activity.