Re: [PEPr] +1 for Text::Text_Highlighter
| From: | Stefan Neufeind | Date: | Thu, 20 May 2004 22:04:11 +0000 |
| Subject: | Re: [PEPr] +1 for Text::Text_Highlighter | ||
| References: | 1 | Groups: | php.pear.dev |
| Request: | Send a blank email to pear-dev+get-29476@lists.php.net to get a copy of this message | ||
On 20 May 2004 at 14:48, David Grant wrote:
> PEPr wrote:
> > David Grant (http://pear.php.net/user/djg) has voted +1 on the
> proposal for Text::Text_Highlighter.
> >
> > Proposal information:
> > http://pear.php.net/pepr/pepr-proposal-show.php?id=72
> > Vote information:
> >
> > http://pear.php.net/pepr/pepr-vote-show.php?id=72&handle=djg
> >
>
> Erm, I just noticed something in Highlighter.php. On line 168, you're
> using single quotes for the string, which means variable substitution
> won't happen. This means I get the following misleading error string:
>
> "Highlighter for ${lang} not found"
>
> Instead of (e.g.)
>
> "Highlighter for FOOBAR not found"
>
> To reproduce:
>
> require_once("Text/Highlighter.php");
> var_dump(Highlighter::factory("FOOBAR"));
Imho it might be a good idea to be more clear about it and use
'abc'.$lang.'def' - just stop and start the string again. makes it
more readable and less failure-prone. If you're using things like
"abc $foo_bar def" or even
"abc-$foo_bat-def" it might get quite confusing otherwise.
Stefan