improved error message accuracy in Mojo::Template by using line direc…

…tives
mojolicious · Nov 12, 2012 · 99b1521 · 99b1521
1 parent 721dbe9
commit 99b1521
Show file tree

Hide file tree

Showing 7 changed files with 29 additions and 57 deletions.
diff --git a/Changes b/Changes
@@ -1,5 +1,7 @@
 
-3.57  2012-11-10
+3.57  2012-11-12
+  - Deprecated Mojo::Exception->raw_message.
+  - Improved error message accuracy in Mojo::Template by using line diretives.
   - Improved performance of contains method in Mojo::Asset::File by 100%.
   - Improved tests.
   - Fixed range bug in Mojo::Asset::Memory.

diff --git a/lib/Mojo/Exception.pm b/lib/Mojo/Exception.pm
@@ -8,14 +8,20 @@ use overload
 use Scalar::Util 'blessed';
 
 has [qw(frames line lines_before lines_after)] => sub { [] };
-has [qw(message raw_message)] => 'Exception!';
+has message => 'Exception!';
 has verbose => sub { $ENV{MOJO_EXCEPTION_VERBOSE} || 0 };
 
 sub new {
   my $self = shift->SUPER::new;
   return @_ ? $self->_detect(@_) : $self;
 }
 
+# DEPRECATED in Rainbow!
+sub raw_message {
+  warn "Mojo::Exception->raw_message has been DEPRECATED!\n";
+  shift->message(@_);
+}
+
 sub throw { die shift->new->trace(2)->_detect(@_) }
 
 sub to_string {
@@ -91,7 +97,7 @@ sub _detect {
   # Message
   my $msg = shift;
   return $msg if blessed $msg && $msg->isa('Mojo::Exception');
-  $self->message($msg)->raw_message($msg);
+  $self->message($msg);
 
   # Extract file and line from message
   my @trace;
@@ -101,7 +107,7 @@ sub _detect {
   my $first = $self->frames->[0];
   unshift @trace, [$first->[1], $first->[2]] if $first && $first->[1];
 
-  # Search for context
+  # Search for context in files
   for my $frame (reverse @trace) {
     next unless -r $frame->[0];
     open my $handle, '<:utf8', $frame->[0];
@@ -112,30 +118,7 @@ sub _detect {
   # More context
   return $self unless my $files = shift;
   my @lines = map { [split /\n/] } @$files;
-
-  # Fix file in message
-  return $self unless my $name = shift;
-  unless (ref $msg) {
-    my $filter = sub {
-      my $num  = shift;
-      my $new  = "$name line $num";
-      my $line = $lines[0][$num];
-      return defined $line ? qq{$new, near "$line".} : "$new.";
-    };
-    $msg =~ s/\(eval\s+\d+\) line (\d+).*/$filter->($1)/ge;
-    $self->message($msg);
-  }
-
-  # Search for better context
-  my $line;
-  if ($self->message =~ /at\s+\Q$name\E\s+line\s+(\d+)/) { $line = $1 }
-  else {
-    for my $frame (@{$self->frames}) {
-      $line = $frame->[1] =~ /^\(eval \d+\)$/ ? $frame->[2] : next;
-      last;
-    }
-  }
-  $self->_context($line, \@lines) if $line;
+  $self->_context($trace[0][1], \@lines) if $trace[0][1];
 
   return $self;
 }
@@ -199,13 +182,6 @@ Lines before the line where the exception occured.
 
 Exception message.
 
-=head2 C<raw_message>
-
-  my $msg = $e->raw_message;
-  $e      = $e->raw_message('Oops!');
-
-Raw unprocessed exception message.
-
 =head2 C<verbose>
 
   my $verbose = $e->verbose;
@@ -222,14 +198,14 @@ following new ones.
 =head2 C<new>
 
   my $e = Mojo::Exception->new('Oops!');
-  my $e = Mojo::Exception->new('Oops!', $files, $name);
+  my $e = Mojo::Exception->new('Oops!', $files);
 
 Construct a new L<Mojo::Exception> object.
 
 =head2 C<throw>
 
   Mojo::Exception->throw('Oops!');
-  Mojo::Exception->throw('Oops!', $files, $name);
+  Mojo::Exception->throw('Oops!', $files);
 
 Throw exception with stacktrace.
 

diff --git a/lib/Mojo/Template.pm b/lib/Mojo/Template.pm
@@ -100,14 +100,13 @@ sub build {
 sub compile {
   my $self = shift;
 
-  # Compile
+  # Compile with line directives
   return undef unless my $code = $self->code;
-  my $compiled = eval $code;
+  my $compiled = eval qq{#line 1 "@{[$self->name]}"\n} . $code;
   $self->compiled($compiled) and return undef unless $@;
 
   # Use local stacktrace for compile exceptions
-  return Mojo::Exception->new($@, [$self->template, $code], $self->name)
-    ->trace->verbose(1);
+  return Mojo::Exception->new($@, [$self->template, $code])->trace->verbose(1);
 }
 
 sub interpret {
@@ -116,7 +115,7 @@ sub interpret {
   # Stacktrace
   local $SIG{__DIE__} = sub {
     CORE::die($_[0]) if ref $_[0];
-    Mojo::Exception->throw(shift, [$self->template, $self->code], $self->name);
+    Mojo::Exception->throw(shift, [$self->template, $self->code]);
   };
 
   # Interpret
@@ -125,7 +124,7 @@ sub interpret {
   return $output unless $@;
 
   # Exception
-  return Mojo::Exception->new($@, [$self->template], $self->name)->verbose(1);
+  return Mojo::Exception->new($@, [$self->template])->verbose(1);
 }
 
 sub parse {

diff --git a/lib/Mojolicious/Plugin/EPLRenderer.pm b/lib/Mojolicious/Plugin/EPLRenderer.pm
@@ -35,7 +35,6 @@ sub _epl {
     # Inline
     if (defined $inline) {
       $c->app->log->debug('Rendering inline template.');
-      $mt->name('inline template');
       $$output = $mt->render($inline, $c);
     }
 
@@ -47,15 +46,13 @@ sub _epl {
       # Try template
       if (-r $path) {
         $c->app->log->debug(qq{Rendering template "$t".});
-        $mt->name(qq{template "$t"});
-        $$output = $mt->render_file($path, $c);
+        $$output = $mt->name($t)->render_file($path, $c);
       }
 
       # Try DATA section
       elsif (my $d = $renderer->get_data_template($options)) {
         $c->app->log->debug(qq{Rendering template "$t" from DATA section.});
-        $mt->name(qq{template "$t" from DATA section});
-        $$output = $mt->render($d, $c);
+        $$output = $mt->name($t)->render($d, $c);
       }
 
       # No template

diff --git a/t/mojo/template.t b/t/mojo/template.t
@@ -673,8 +673,7 @@ is $output->lines_after->[0][0], 4,          'right number';
 is $output->lines_after->[0][1], '%= 1 + 1', 'right line';
 is $output->lines_after->[1][0], 5,          'right number';
 is $output->lines_after->[1][1], 'test',     'right line';
-like "$output", qr/oops! at template line 3, near "%= 1 \+ 1"./,
-  'right result';
+like "$output", qr/oops! at template line 3/, 'right result';
 
 # Exception in template (empty perl lines)
 $mt     = Mojo::Template->new;
@@ -711,7 +710,7 @@ is $output->lines_after->[1][2], ' ',   'right code';
 is $output->lines_after->[2][0], 7,     'right number';
 is $output->lines_after->[2][1], '%',   'right line';
 is $output->lines_after->[2][2], ' ',   'right code';
-like "$output", qr/oops! at template line 4, near "%"./, 'right result';
+like "$output", qr/oops! at template line 4/, 'right result';
 
 # Exception in nested template
 $mt = Mojo::Template->new;

diff --git a/t/mojolicious/lite_app.t b/t/mojolicious/lite_app.t
@@ -683,21 +683,19 @@ $t->get_ok('/reserved?data=just-works&json=test')->status_is(200)
 $t->get_ok('/inline/exception')->status_is(500)
   ->header_is(Server         => 'Mojolicious (Perl)')
   ->header_is('X-Powered-By' => 'Mojolicious (Perl)')
-  ->content_is("Died at inline template line 1.\n\n");
+  ->content_is("Died at template line 1.\n\n");
 
 # GET /data/exception
 $t->get_ok('/data/exception')->status_is(500)
   ->header_is(Server         => 'Mojolicious (Perl)')
   ->header_is('X-Powered-By' => 'Mojolicious (Perl)')
-  ->content_is(qq{Died at template "dies.html.ep" from DATA section line 2}
-    . qq{, near "123".\n\n});
+  ->content_is("Died at dies.html.ep line 2.\n\n");
 
 # GET /template/exception
 $t->get_ok('/template/exception')->status_is(500)
   ->header_is(Server         => 'Mojolicious (Perl)')
   ->header_is('X-Powered-By' => 'Mojolicious (Perl)')
-  ->content_is(
-  qq{Died at template "dies_too.html.ep" line 2, near "321".\n\n});
+  ->content_is("Died at dies_too.html.ep line 2.\n\n");
 
 # GET /with-format
 $t->get_ok('/with-format')->content_is("/without-format\n");

diff --git a/t/pod_coverage.t b/t/pod_coverage.t
@@ -7,4 +7,5 @@ plan skip_all => 'set TEST_POD to enable this test (developer only!)'
 plan skip_all => 'Test::Pod::Coverage 1.04 required for this test!'
   unless eval 'use Test::Pod::Coverage 1.04; 1';
 
-all_pod_coverage_ok();
+# DEPRECATED in Rainbow!
+all_pod_coverage_ok({also_private => ['raw_message']});