Edgewall Software
Modify

Ticket #3261 (closed defect: fixed)

Opened 6 years ago

Last modified 6 years ago

PHP highlighter treats content as string (really a stream)

Reported by: Tim Hatch <trac@…> Owned by: mgood
Priority: normal Milestone: 0.10
Component: version control/browser Version: devel
Severity: normal Keywords:
Cc:
Release Notes:
API Changes:

Description

I noticed this while tracking down another php-related rendering bug. The PHP renderer is never being used on my system because it triggers an exception. The content passed into render() (mimeview/php.py) is a stream object, not a string. Stream objects don't seem to include the encode member, but if you toss a .read() in there before encode it works fine (and simpler than wrapping it with an encoder instance). Attaching the simple patch in a sec.

2006-06-13 08:57:51,381 Trac[api] DEBUG: Trying to render HTML preview using PHPRenderer
2006-06-13 08:57:51,384 Trac[php] DEBUG: PHP command line: php -sn
2006-06-13 08:57:51,385 Trac[api] WARNING: HTML preview using <trac.mimeview.php.PHPRenderer object at 0x412511ac> failed (Stream instance has no attribute 'encode')
Traceback (most recent call last):
  File "/usr/lib/python2.3/site-packages/trac/mimeview/api.py", line 402, in render
    return Markup(self._annotate(result, annotations))
  File "/usr/lib/python2.3/site-packages/trac/mimeview/api.py", line 436, in _annotate
    for num, line in enumerate(_html_splitlines(lines)):
  File "/usr/lib/python2.3/site-packages/trac/mimeview/api.py", line 566, in _html_splitlines
    for line in lines:
  File "/usr/lib/python2.3/site-packages/trac/mimeview/php.py", line 67, in render
    np = NaivePopen(cmdline, content.encode('utf-8'), capturestderr=1)
AttributeError: Stream instance has no attribute 'encode'
2006-06-13 08:57:51,386 Trac[api] DEBUG: Trying to render HTML preview using SilverCityRenderer

Attachments

php-content-is-stream.diff (620 bytes) - added by Tim Hatch <trac@…> 6 years ago.
diff against trunk@3397
mimeview-php-unicode-stream.diff (1.0 KB) - added by Tim Hatch <trac@…> 6 years ago.
Diff against trunk@3423 for a more complete fix based off r3421

Download all attachments as: .zip

Change History

Changed 6 years ago by Tim Hatch <trac@…>

diff against trunk@3397

comment:1 Changed 6 years ago by Tim Hatch <trac@…>

Actually this bug is a little more complex. My quick fix only works for ascii source files. If (valid) utf-8 exists in the file, even if utf-8 is set as a default_charset in trac.ini, it chokes on a UnicodeDecodeError.

comment:2 Changed 6 years ago by mgood

If the content is a stream then I don't think the call to encode is necessary. It would've been necessary if content was a unicode object, but the result of content.read() should already be an encoded string.

However, based on the docstring for IHTMLPreviewRenderer the content can be a str or unicode object or a file-like object with a read method. So the PHP renderer should be made to work with any of those inputs.

comment:3 Changed 6 years ago by mgood

  • Owner changed from jonas to mgood

It should be pretty easy to update it to work with any of those inputs, so I'll commit a fix for this later.

comment:4 Changed 6 years ago by mgood

  • Milestone set to 0.10

Changed 6 years ago by Tim Hatch <trac@…>

Diff against trunk@3423 for a more complete fix based off r3421

comment:5 Changed 6 years ago by Tim Hatch <trac@…>

I noticed some changes going in for the other preview renderers. The fix in mimeview-php-unicode-stream.diff uses the same general method, and renders correctly for the files I tested.

comment:6 Changed 6 years ago by cboos

  • Resolution set to fixed
  • Status changed from new to closed

I applied your patch in r3424, thanks!

View

Add a comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
The resolution will be deleted. Next status will be 'reopened'
to The owner will be changed from mgood. Next status will be 'closed'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.