* [ruby-core:110841] [Ruby master Feature#19138] `SyntaxError#path` for syntax_suggest
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)
2022-11-22 23:07 ` [ruby-core:110862] " Eregon (Benoit Daloze)
` (4 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: nobu (Nobuyoshi Nakada) @ 2022-11-21 16:08 UTC (permalink / raw)
To: ruby-core
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/
^ permalink raw reply related [flat|nested] 7+ messages in thread* [ruby-core:110862] [Ruby master Feature#19138] `SyntaxError#path` for syntax_suggest
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 ` [ruby-core:110841] [Ruby master Feature#19138] " nobu (Nobuyoshi Nakada)
@ 2022-11-22 23:07 ` Eregon (Benoit Daloze)
2022-11-22 23:14 ` [ruby-core:110863] " nobu (Nobuyoshi Nakada)
` (3 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Eregon (Benoit Daloze) @ 2022-11-22 23:07 UTC (permalink / raw)
To: ruby-core
Issue #19138 has been updated by Eregon (Benoit Daloze).
Should we also expose the `line`. Or maybe `source_location`?
Just a suggestion, if we only need `path` I think it's fine to just add `SyntaxError#path`.
----------------------------------------
Feature #19138: `SyntaxError#path` for syntax_suggest
https://bugs.ruby-lang.org/issues/19138#change-100222
* 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/
^ permalink raw reply related [flat|nested] 7+ messages in thread* [ruby-core:110863] [Ruby master Feature#19138] `SyntaxError#path` for syntax_suggest
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 ` [ruby-core:110841] [Ruby master Feature#19138] " nobu (Nobuyoshi Nakada)
2022-11-22 23:07 ` [ruby-core:110862] " Eregon (Benoit Daloze)
@ 2022-11-22 23:14 ` nobu (Nobuyoshi Nakada)
2022-11-26 0:21 ` [ruby-core:111015] " schneems (Richard Schneeman)
` (2 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: nobu (Nobuyoshi Nakada) @ 2022-11-22 23:14 UTC (permalink / raw)
To: ruby-core
Issue #19138 has been updated by nobu (Nobuyoshi Nakada).
`SyntaxError` can have a series of errors.
Before adding `line`, we need to consider how to provide that list.
----------------------------------------
Feature #19138: `SyntaxError#path` for syntax_suggest
https://bugs.ruby-lang.org/issues/19138#change-100223
* 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/
^ permalink raw reply related [flat|nested] 7+ messages in thread* [ruby-core:111015] [Ruby master Feature#19138] `SyntaxError#path` for syntax_suggest
2022-11-21 3:18 [ruby-core:110828] [Ruby master Bug#19138] `SyntaxError#path` for syntax_suggest nobu (Nobuyoshi Nakada)
` (2 preceding siblings ...)
2022-11-22 23:14 ` [ruby-core:110863] " nobu (Nobuyoshi Nakada)
@ 2022-11-26 0:21 ` 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)
5 siblings, 0 replies; 7+ messages in thread
From: schneems (Richard Schneeman) @ 2022-11-26 0:21 UTC (permalink / raw)
To: ruby-core
Issue #19138 has been updated by schneems (Richard Schneeman).
I love the idea.
Instead of adding #line though if we could attach the source that would be more impactful for syntax search.
Some cases such as eval do not have source files, so if we could access the contents for casses without a path that would increase the capabilities of the gem.
----------------------------------------
Feature #19138: `SyntaxError#path` for syntax_suggest
https://bugs.ruby-lang.org/issues/19138#change-100269
* 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/
^ permalink raw reply related [flat|nested] 7+ messages in thread* [ruby-core:111018] [Ruby master Feature#19138] `SyntaxError#path` for syntax_suggest
2022-11-21 3:18 [ruby-core:110828] [Ruby master Bug#19138] `SyntaxError#path` for syntax_suggest nobu (Nobuyoshi Nakada)
` (3 preceding siblings ...)
2022-11-26 0:21 ` [ruby-core:111015] " schneems (Richard Schneeman)
@ 2022-11-26 12:40 ` Eregon (Benoit Daloze)
2022-12-01 8:05 ` [ruby-core:111114] " matz (Yukihiro Matsumoto)
5 siblings, 0 replies; 7+ messages in thread
From: Eregon (Benoit Daloze) @ 2022-11-26 12:40 UTC (permalink / raw)
To: ruby-core
Issue #19138 has been updated by Eregon (Benoit Daloze).
schneems (Richard Schneeman) wrote in #note-6:
> Instead of adding #line though if we could attach the source that would be more impactful for syntax search.
>
> Some cases such as eval do not have source files, so if we could access the contents for casses without a path that would increase the capabilities of the gem.
Strongly agreed. We should have a way to get the original source code/text from an exception or a Method/UnboundMethod object, or any core object which has a `source_location`.
The current way trying to reread from disk is quite brittle and incomplete, as seen for `error_highlight` by @mame.
TruffleRuby already keeps this information, it's only a matter of exposing it through some method.
That should be a separate feature request though.
----------------------------------------
Feature #19138: `SyntaxError#path` for syntax_suggest
https://bugs.ruby-lang.org/issues/19138#change-100273
* 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/
______________________________________________
ruby-core mailing list -- ruby-core@ml.ruby-lang.org
To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
ruby-core info -- https://ml.ruby-lang.org/mailman3/postorius/lists/ruby-core.ml.ruby-lang.org/
^ permalink raw reply related [flat|nested] 7+ messages in thread* [ruby-core:111114] [Ruby master Feature#19138] `SyntaxError#path` for syntax_suggest
2022-11-21 3:18 [ruby-core:110828] [Ruby master Bug#19138] `SyntaxError#path` for syntax_suggest nobu (Nobuyoshi Nakada)
` (4 preceding siblings ...)
2022-11-26 12:40 ` [ruby-core:111018] " Eregon (Benoit Daloze)
@ 2022-12-01 8:05 ` matz (Yukihiro Matsumoto)
5 siblings, 0 replies; 7+ messages in thread
From: matz (Yukihiro Matsumoto) @ 2022-12-01 8:05 UTC (permalink / raw)
To: ruby-core
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/
______________________________________________
ruby-core mailing list -- ruby-core@ml.ruby-lang.org
To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
ruby-core info -- https://ml.ruby-lang.org/mailman3/postorius/lists/ruby-core.ml.ruby-lang.org/
^ permalink raw reply related [flat|nested] 7+ messages in thread