From: "nobu (Nobuyoshi Nakada)" <noreply@ruby-lang.org>
To: ruby-core@neon.ruby-lang.org
Subject: [ruby-core:110841] [Ruby master Feature#19138] `SyntaxError#path` for syntax_suggest
Date: Mon, 21 Nov 2022 16:08:58 +0000 (UTC) [thread overview]
Message-ID: <redmine.journal-100197.20221121160857.4@ruby-lang.org> (raw)
In-Reply-To: redmine.issue-19138.20221121031804.4@ruby-lang.org
Issue #19138 has been updated by nobu (Nobuyoshi Nakada).
https://github.com/ruby/ruby/pull/6779
----------------------------------------
Feature #19138: `SyntaxError#path` for syntax_suggest
https://bugs.ruby-lang.org/issues/19138#change-100197
* Author: nobu (Nobuyoshi Nakada)
* Status: Open
* Priority: Normal
----------------------------------------
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/
next prev parent reply other threads:[~2022-11-21 16:09 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-11-21 3:18 [ruby-core:110828] [Ruby master Bug#19138] `SyntaxError#path` for syntax_suggest nobu (Nobuyoshi Nakada)
2022-11-21 16:08 ` nobu (Nobuyoshi Nakada) [this message]
2022-11-22 23:07 ` [ruby-core:110862] [Ruby master Feature#19138] " Eregon (Benoit Daloze)
2022-11-22 23:14 ` [ruby-core:110863] " nobu (Nobuyoshi Nakada)
2022-11-26 0:21 ` [ruby-core:111015] " schneems (Richard Schneeman)
2022-11-26 12:40 ` [ruby-core:111018] " Eregon (Benoit Daloze)
2022-12-01 8:05 ` [ruby-core:111114] " matz (Yukihiro Matsumoto)
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://www.ruby-lang.org/en/community/mailing-lists/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=redmine.journal-100197.20221121160857.4@ruby-lang.org \
--to=ruby-core@ruby-lang.org \
--cc=ruby-core@neon.ruby-lang.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).