Feature #19138
Updated by nobu (Nobuyoshi Nakada) over 1 year ago
Currently syntax_suggest searches the path name from the exception message.
But extracting the info from messages for humans is fragile, I think.
So proposing a new method `SyntaxError#path`, similar to `LoadError#path`.
```patch
commit 986da132002af1cdb75c0c89ca2831fe51e6ce69
Author: Nobuyoshi Nakada <nobu@ruby-lang.org>
AuthorDate: 2022-11-20 22:59:52 +0900
Commit: Nobuyoshi Nakada <nobu@ruby-lang.org>
CommitDate: 2022-11-20 23:44:27 +0900
Add `SyntaxError#path`
diff --git a/error.c b/error.c
index 0ff4b8d6d8e..ad1bc6ee8dc 100644
--- a/error.c
+++ b/error.c
@@ -125,6 +125,8 @@ err_vcatf(VALUE str, const char *pre, const char *file, int line,
return str;
}
+static VALUE syntax_error_with_path(VALUE, VALUE, VALUE*, rb_encoding*);
+
VALUE
rb_syntax_error_append(VALUE exc, VALUE file, int line, int column,
rb_encoding *enc, const char *fmt, va_list args)
@@ -138,15 +140,7 @@ rb_syntax_error_append(VALUE exc, VALUE file, int line, int column,
}
else {
VALUE mesg;
- if (NIL_P(exc)) {
- mesg = rb_enc_str_new(0, 0, enc);
- exc = rb_class_new_instance(1, &mesg, rb_eSyntaxError);
- }
- else {
- mesg = rb_attr_get(exc, idMesg);
- if (RSTRING_LEN(mesg) > 0 && *(RSTRING_END(mesg)-1) != '\n')
- rb_str_cat_cstr(mesg, "\n");
- }
+ exc = syntax_error_with_path(exc, file, &mesg, enc);
err_vcatf(mesg, NULL, fn, line, fmt, args);
}
@@ -2353,6 +2347,25 @@ syntax_error_initialize(int argc, VALUE *argv, VALUE self)
return rb_call_super(argc, argv);
}
+static VALUE
+syntax_error_with_path(VALUE exc, VALUE path, VALUE *mesg, rb_encoding *enc)
+{
+ if (NIL_P(exc)) {
+ *mesg = rb_enc_str_new(0, 0, enc);
+ exc = rb_class_new_instance(1, mesg, rb_eSyntaxError);
+ rb_ivar_set(exc, id_i_path, path);
+ }
+ else {
+ if (rb_attr_get(exc, id_i_path) != path) {
+ rb_raise(rb_eArgError, "SyntaxError#path changed");
+ }
+ VALUE s = *mesg = rb_attr_get(exc, idMesg);
+ if (RSTRING_LEN(s) > 0 && *(RSTRING_END(s)-1) != '\n')
+ rb_str_cat_cstr(s, "\n");
+ }
+ return exc;
+}
+
/*
* Document-module: Errno
*
@@ -3011,9 +3024,14 @@ Init_Exception(void)
rb_eSyntaxError = rb_define_class("SyntaxError", rb_eScriptError);
rb_define_method(rb_eSyntaxError, "initialize", syntax_error_initialize, -1);
+ ID id_path = rb_intern_const("path");
+
+ /* the path failed to parse */
+ rb_attr(rb_eSyntaxError, id_path, TRUE, FALSE, FALSE);
+
rb_eLoadError = rb_define_class("LoadError", rb_eScriptError);
/* the path failed to load */
- rb_attr(rb_eLoadError, rb_intern_const("path"), TRUE, FALSE, FALSE);
+ rb_attr(rb_eLoadError, id_path, TRUE, FALSE, FALSE);
rb_eNotImpError = rb_define_class("NotImplementedError", rb_eScriptError);
```
With this method, syntax_suggest/core_ext.rb will no longer need `PathnameFromMessage`.
```patch
diff --git i/lib/syntax_suggest/core_ext.rb w/lib/syntax_suggest/core_ext.rb
index 40f5fe13759..616a6ed9839 100644
--- i/lib/syntax_suggest/core_ext.rb
+++ w/lib/syntax_suggest/core_ext.rb
@@ -25,15 +25,12 @@
require "syntax_suggest/api" unless defined?(SyntaxSuggest::DEFAULT_VALUE)
message = super
- file = if highlight
- SyntaxSuggest::PathnameFromMessage.new(super(highlight: false, **kwargs)).call.name
- else
- SyntaxSuggest::PathnameFromMessage.new(message).call.name
- end
-
- io = SyntaxSuggest::MiniStringIO.new
+ file = path
if file
+ file = Pathname.new(file)
+ io = SyntaxSuggest::MiniStringIO.new
+
SyntaxSuggest.call(
io: io,
source: file.read,
```
Since we have not released with `SyntaxError#detailed_message` yet, there should not be a compatibility issue.
@schneems How do you think?