error.c: Refactoring

Factor out from rb_error_write the responsibility to check if stderr is
a tty.

error.c

@@ -1247,6 +1247,60 @@ exc_s_to_tty_p(VALUE self)
     return RBOOL(rb_stderr_tty_p());
 }
 
+static VALUE
+check_highlight_keyword(VALUE opt)
+{
+    VALUE highlight = Qnil;
+
+    if (!NIL_P(opt)) {
+        static VALUE kw_highlight;
+        if (!kw_highlight) kw_highlight = ID2SYM(rb_intern_const("highlight"));
+
+        highlight = rb_hash_aref(opt, kw_highlight);
+
+        switch (highlight) {
+          default:
+            rb_bool_expected(highlight, "highlight");
+            UNREACHABLE;
+          case Qundef: highlight = Qnil; break;
+          case Qtrue: case Qfalse: case Qnil: break;
+        }
+    }
+
+    if (NIL_P(highlight)) {
+	highlight = rb_stderr_tty_p() ? Qtrue : Qfalse;
+    }
+
+    return highlight;
+}
+
+static VALUE
+check_order_keyword(VALUE opt)
+{
+    VALUE order = Qnil;
+
+    if (!NIL_P(opt)) {
+        static VALUE kw_order;
+        if (!kw_order) kw_order = ID2SYM(rb_intern_const("order"));
+
+        order = rb_hash_aref(opt, kw_order);
+
+        if (order != Qnil) {
+            ID id = rb_check_id(&order);
+            if (id == id_bottom) order = Qtrue;
+            else if (id == id_top) order = Qfalse;
+            else {
+                rb_raise(rb_eArgError, "expected :top or :bottom as "
+                        "order: %+"PRIsVALUE, order);
+            }
+        }
+    }
+
+    if (NIL_P(order)) order = Qfalse;
+
+    return order;
+}
+
 /*
  * call-seq:
  *   exception.full_message(highlight: bool, order: [:top or :bottom]) ->  string
@@ -1269,44 +1323,18 @@ static VALUE
 exc_full_message(int argc, VALUE *argv, VALUE exc)
 {
     VALUE opt, str, emesg, errat;
-    enum {kw_highlight, kw_order, kw_max_};
+    VALUE highlight, order;
-    static ID kw[kw_max_];
-    VALUE args[kw_max_] = {Qnil, Qnil};
 
     rb_scan_args(argc, argv, "0:", &opt);
-    if (!NIL_P(opt)) {
+
-	if (!kw[0]) {
+    highlight = check_highlight_keyword(opt);
-#define INIT_KW(n) kw[kw_##n] = rb_intern_const(#n)
+    order = check_order_keyword(opt);
-	    INIT_KW(highlight);
+
-	    INIT_KW(order);
-#undef INIT_KW
-	}
-	rb_get_kwargs(opt, kw, 0, kw_max_, args);
-	switch (args[kw_highlight]) {
-	  default:
-            rb_bool_expected(args[kw_highlight], "highlight");
-	    UNREACHABLE;
-	  case Qundef: args[kw_highlight] = Qnil; break;
-	  case Qtrue: case Qfalse: case Qnil: break;
-	}
-	if (args[kw_order] == Qundef) {
-	    args[kw_order] = Qnil;
-	}
-	else {
-	    ID id = rb_check_id(&args[kw_order]);
-	    if (id == id_bottom) args[kw_order] = Qtrue;
-	    else if (id == id_top) args[kw_order] = Qfalse;
-	    else {
-		rb_raise(rb_eArgError, "expected :top or :bottom as "
-			 "order: %+"PRIsVALUE, args[kw_order]);
-	    }
-	}
-    }
     str = rb_str_new2("");
     errat = rb_get_backtrace(exc);
     emesg = rb_get_message(exc);
 
-    rb_error_write(exc, emesg, errat, str, args[kw_highlight], args[kw_order]);
+    rb_error_write(exc, emesg, errat, str, highlight, order);
     return str;
 }
 

eval_error.c

@@ -200,6 +200,7 @@ print_errinfo(const VALUE eclass, const VALUE errat, const VALUE emesg, const VA
 		else {
 		    elen -= tail - einfo;
 		    einfo = tail;
+                    write_warn2(str, "\n", 1);
 		    while (elen > 0) {
 			tail = memchr(einfo, '\n', elen);
 			if (!tail || tail > einfo) {
@@ -300,10 +301,10 @@ show_cause(VALUE errinfo, VALUE str, VALUE highlight, VALUE reverse, long backtr
         if (reverse) {
             show_cause(cause, str, highlight, reverse, backtrace_limit, shown_causes);
             print_backtrace(eclass, errat, str, TRUE, backtrace_limit);
-            print_errinfo(eclass, errat, emesg, str, highlight!=0);
+            print_errinfo(eclass, errat, emesg, str, RTEST(highlight));
         }
         else {
-            print_errinfo(eclass, errat, emesg, str, highlight!=0);
+            print_errinfo(eclass, errat, emesg, str, RTEST(highlight));
             print_backtrace(eclass, errat, str, FALSE, backtrace_limit);
             show_cause(cause, str, highlight, reverse, backtrace_limit, shown_causes);
         }
@@ -324,11 +325,6 @@ rb_error_write(VALUE errinfo, VALUE emesg, VALUE errat, VALUE str, VALUE highlig
 	errat = Qnil;
     }
     eclass = CLASS_OF(errinfo);
-    if (NIL_P(reverse)) reverse = Qfalse;
-    if (NIL_P(highlight)) {
-	VALUE tty = (VALUE)rb_stderr_tty_p();
-	if (NIL_P(highlight)) highlight = tty;
-    }
     if (reverse) {
 	static const char traceback[] = "Traceback "
 	    "(most recent call last):\n";
@@ -336,7 +332,7 @@ rb_error_write(VALUE errinfo, VALUE emesg, VALUE errat, VALUE str, VALUE highlig
 	char buff[sizeof(traceback)+sizeof(bold)+sizeof(reset)-2], *p = buff;
 	const char *msg = traceback;
 	long len = sizeof(traceback) - 1;
-	if (highlight) {
+	if (RTEST(highlight)) {
 #define APPEND(s, l) (memcpy(p, s, l), p += (l))
 	    APPEND(bold, sizeof(bold)-1);
 	    APPEND(traceback, bold_part);
@@ -348,10 +344,10 @@ rb_error_write(VALUE errinfo, VALUE emesg, VALUE errat, VALUE str, VALUE highlig
 	write_warn2(str, msg, len);
         show_cause(errinfo, str, highlight, reverse, backtrace_limit, &shown_causes);
 	print_backtrace(eclass, errat, str, TRUE, backtrace_limit);
-	print_errinfo(eclass, errat, emesg, str, highlight!=0);
+	print_errinfo(eclass, errat, emesg, str, RTEST(highlight));
     }
     else {
-	print_errinfo(eclass, errat, emesg, str, highlight!=0);
+	print_errinfo(eclass, errat, emesg, str, RTEST(highlight));
 	print_backtrace(eclass, errat, str, FALSE, backtrace_limit);
         show_cause(errinfo, str, highlight, reverse, backtrace_limit, &shown_causes);
     }
@@ -380,7 +376,8 @@ rb_ec_error_print(rb_execution_context_t * volatile ec, volatile VALUE errinfo)
 
     if (!written) {
         written = true;
-        rb_error_write(errinfo, emesg, errat, Qnil, Qnil, Qfalse);
+        VALUE highlight = rb_stderr_tty_p() ? Qtrue : Qfalse;
+        rb_error_write(errinfo, emesg, errat, Qnil, highlight, Qfalse);
     }
 
     EC_POP_TAG();