Commit 3d97c7f broke the output of pm-updatecode, which was using the file argument that said patch removed. I fixed this in master with commit a088dee.
This patch was backported to the 4.x branch with commit 73d6daa. This should not have been done, because it changed the file signature of drush_print_table, which might break external drush commands.
There are a couple of potential solutions to this situation. The change is an improvement, and helpful to the unit tests, so we probably don't want to just back it out. One compatible change that could be made would be to check to see if the 'handle' parameter is a string, and if it is, then assume it is a file path and open up a new handle in 'append' mode. It would also be necessary to insure that the file handle was fclose'd at the end iff it was open. This would maintain compatibility and also preserve the feature.
The code on master is an improvement over what was there before, so I think the above suggestion, if done at all, would only be needed on the 4.x branch. I suppose there would be no harm in doing it in master too, but I don't think it necessary.
| Comment | File | Size | Author |
|---|---|---|---|
| #3 | 1171182-drush-print-table-fix-3.patch | 632 bytes | msonnabaum |
| #2 | 1171182-drush-print-table-fix.patch | 637 bytes | msonnabaum |
Comments
Comment #1
greg.1.anderson commentedClarification: when I said "this patch was backported" above, I meant the removal of the file arg from drush_print_table. I did not backport my fix to pm-updatecode.
Comment #2
msonnabaum commentedSorry about that. Moshe and I hashed this one out on IRC and I thought we determined it wasn't being used. Should we maybe add a test that would have caught this? I'm still not 100% clear on how updatecode uses it.
I think the approach you suggested sounds good. Here's a patch that should work.
Comment #3
msonnabaum commentedI see now where the update-code test throws warnings about this. New patch appears to be working for me.
Comment #4
greg.1.anderson commentedYeah, that works great.
Comment #5
msonnabaum commentedCommitted.