Wrap stdout at 80 characters (where reasonable)
| Project: | Drush |
| Version: | All-Versions-HEAD |
| Component: | Code |
| Category: | feature request |
| Priority: | minor |
| Assigned: | Owen Barton |
| Status: | active |
| Issue tags: | commandline |
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.

#1
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.
#2
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).
#3
My machine definitely sets a $COLUMNS variable. On my Debian laptop I can do this:
0 nat@pigtown:~$ echo $COLUMNS80
Then I change my terminal from 80, to full screen width, and run it again:
0 nat@pigtown:~$ echo $COLUMNS166
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.
#4
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 :
<?phpphp -r "print `echo $COLUMNS`;"
?>
We'll fix this soon enough.
#5
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().
#6
I would really like to see this before final 2.0 release.
#7
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.
#8
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).
<?php$x = `echo \$HOME`;
print($x);
/Users/adrian
$x = `echo \$COLUMNS`;
print($x);
?>
Notice the last line is blank.
#9
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.
<?php
Moya:~ adrian$ sh -l -c 'echo $COLUMNS';
Moya:~ adrian$ export COLUMNS=$COLUMNS;
Moya:~ adrian$ sh -l -c 'echo $COLUMNS';
175
?>
#10
I tried. even in a standard bash shell script, the $columns variable is always unavailable, unless it has been exported.
#11
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
#12
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
#13
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.
#14
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.
#15
i found a better mechanism than columns, if it doesn't exist.
<?php>> system('stty size');
46 148
?>
dunno if it's available on windows.
#16
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
#17
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.
#18
Minor tweaks over the last one ... You can test these with the commands `drush help`, drush help dl, and drush watchdog show.
#19
lets see what folks think of the 2 patches. feedback wanted. neither of us is thrilled with our patches.
#20
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 blahbleh quux thing.
You would do:
drush foo bardo foo bar stuff on thing blah
bleh quux thing.
Easier to wordwrap, cleaner...
#21
and doubles the number of lines. thats not acceptable for a CLI app IMO.
#22
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...
#23
#24
personally. i feel this shouldn't hold up release.
#25
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.
#26
Minor fix for table headers.
#27
Patch works well.
#28
Ok. on closer inspection :
The patch to command.inc looks like gunk from http://drupal.org/node/418208
<?phpIndex: 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.
#29
Patch without sneaky impostor chunk.
#30
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.
#31
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).
#32
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.
#33
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.
#34
Added both wget and curl fallbacks
#35
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.
#36
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).
#37
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.
#38
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 aview 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.
#39
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).
#40
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!
#41
Same here, everything's working now though
#42
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. :)
#43
of course yes. patches welcome. thanks.
#44
@Senpai - what was the error Drush was giving when it tried to auto-download table.inc?
#45
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
#46
Well spotted, I think that explains the issues here.
Committed - thanks!
#47
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.
#48
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:
#49
This patch applies OK and gives a better error message
Tested on environment.inc v 1.37 in HEAD
#50
Committed - thanks!
#51
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.
#52
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.#53
Also @naught101: to disable wrapping you could do something like
$ env COLUMNS=999 drush statusmodulesThough a --nowrap switch would definitely be more convenient.
#54
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?
#55
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.
#56
When I'm running drush as a shell command under vim, eg using the Ex command
:r !drush statusmodulesI'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 colswill give this output: