Issue #19138 has been updated by matz (Yukihiro Matsumoto). Sounds reasonable. Matz. ---------------------------------------- Feature #19138: `SyntaxError#path` for syntax_suggest https://bugs.ruby-lang.org/issues/19138#change-100386 * Author: nobu (Nobuyoshi Nakada) * Status: Open * Priority: Normal * Target version: 3.2 ---------------------------------------- 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? -- https://bugs.ruby-lang.org/