I have set COLUMNS=132 in our environment, which causes Drush to fix the screen width at 132.

As shown in this example, that does not get processed until the third [bootstrap] message. Can it be adjusted to be read prior to all logging?

X-Powered-By: PHP/5.2.17
Content-type: text/html

Bootstrap to phase 0. [0.37 sec, 1.25 MB]                            [bootstrap]
Drush bootstrap phase : _drush_bootstrap_drush() [0.4 sec, 1.38 MB]  [bootstrap]
Bootstrap to phase 6. [1.14 sec, 3 MB]                                                                                   [bootstrap]
CommentFileSizeAuthor
#8 1065150-2.columns-earlier.patch6.19 KBkotnik
#6 1065150.columns-earlier.patch2.89 KBkotnik

Comments

mdlueck’s picture

mdlueck commented

BTW: I confirmed that this was not an issue with Drush 3 / Drupal 6

20110217-004501 drush core-cron cron.php begin
Drush bootstrap phase : _drush_bootstrap_drupal_root() [0.04 sec, 3.87 MB]                                               [bootstrap]
Initialized Drupal 6.20 root directory at /srv/www/sites/domain.org/www [0.04 sec, 4.76 MB]                                 [notice]
Drush bootstrap phase : _drush_bootstrap_drupal_site() [0.05 sec, 4.87 MB]                                               [bootstrap]
greg.1.anderson’s picture

greg.1.anderson commented

Hm, others can weigh in, but my initial reaction is 'will not fix'. The benefit does not seem to be worth the restriction. We would have to do COLUMNS very early to insure that no one added any log message before that point.

@mdlueck: what about the Content-type: text/html in the first output? Is that part of your bug report? I'm guessing that might just be part of your configuration; usually you do not see that when using drush with php-cli.

mdlueck’s picture

mdlueck commented

@greg.1.anderson: I did notice those extra two lines, X-Powered-By and Content-type. I was not bothered by their presence.

I am using Drush to run Drupal Cron via:

echo $DATETIME 'drush core-cron cron.php begin' | /usr/bin/tee -a $LOGFILE
/usr/bin/nice -n19 /usr/local/bin/php5 /kunden/homepages/35/d157794216/htdocs/drush4/drush.php --root=$1 --uri=http://$2 --debug --nocolor core-cron 2>&1 | /usr/bin/tee -a $LOGFILE

So, processing COLUMNS even earlier in the start-up process would be helpful for consistency. Would it be very hard to move that upwards in the Drush startup process?

greg.1.anderson’s picture

greg.1.anderson commented

Logically, it makes sense to process COLUMNS with the drush bootstrap. The issue seems too minor to justify moving the COLUMNS initialization code out-of-sequence to the beginning of drush.php. I'm sort of neutral; maybe other maintainers would take a different view. You could always try it, see if it works okay and doesn't look too ugly, and post a patch.

moshe weitzman’s picture

moshe weitzman commented
Version: All-versions-4.2 »
Category: bug » feature
Priority: Normal » Minor

I'm pretty neutral as well.

kotnik’s picture

kotnik commented
Status: Active » Needs review
StatusFileSize
new 1065150.columns-earlier.patch2.89 KB

Here's a patch that bumps width calculation.

greg.1.anderson’s picture

greg.1.anderson commented
Status: Needs review » Needs work

At least wrap this code in its own function; that's too much to just drop into drush_main().

kotnik’s picture

kotnik commented
Status: Needs work » Needs review
StatusFileSize
new 1065150-2.columns-earlier.patch6.19 KB

Code separated.

moshe weitzman’s picture

moshe weitzman commented
Status: Needs review » Closed (works as designed)

No longer applies cleanly. But yeah, lets just leave this code where it is. I do like its refactoring into own function though