Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve support for warning categories #3508

Merged
merged 2 commits into from
Sep 28, 2020

Conversation

jeremyevans
Copy link
Contributor

This adds support for the category keyword for Kernel#warn (for Ruby level warnings), and adds rb_category_warn and rb_category_warning C-API functions (for C level warnings). It changes Warning.warn to only accept a category keyword, instead of accepting arbitrary keywords. It modifies all of the warnings emitted by core Ruby to use a category.

This also removes some deprecated functions/methods that were scheduled to be removed in Ruby 3.0 that I found while looking for warnings to modify.

@jeremyevans
Copy link
Contributor Author

I've rebased this after eeb5325 and introduction of Ractor to fix conflicts, and squashed a couple of the commits.

@jeremyevans
Copy link
Contributor Author

I've rebased this again to fix a conflict related to the removal of ruby -T. I also removed the commit for removing rb_find_file_ext_safe and rb_find_file_safe, since @hsbt has the same change in #3537.

In general accepting arbitrary keywords is a bad idea unless you are
delegating keywords or acting on arbitrary keywords.  In this case,
the category keyword is ignored, and it's less error prone to not
ignore all keywords.
@jeremyevans jeremyevans force-pushed the warning-categories branch 3 times, most recently from 342d948 to 9ad2e4c Compare September 25, 2020 23:27
This adds the following C-API functions that can be used to emit
warnings with categories included:

```c
void rb_category_warn(const char *, const char*, ...)
void rb_category_warning(const char*, const char*, ...)
```

Internally in error.c, there is an rb_warn_category function
that will call Warning.warn with the string and the category
keyword if it doesn't have an arity of 1, and will call
Warning.warn with just the string if it has an arity of 1.
This refactors the rb_warn_deprecated{,_to_remove} functions
to use rb_warn_category.

This makes Kernel#warn accept a category keyword and pass it
to Warning.warn, so that Ruby methods can more easily emit
warnings with categories.  rb_warn_category makes sure that
the passed category is a already defined category symbol
before calling Warning.warn.

The only currently defined warning category is :deprecated,
since that is what is already used.  More categories can be
added in later commits.
@jeremyevans
Copy link
Contributor Author

I modified the implementation to only allow specific category symbols, as requested by matz. By default, only :deprecated is supported, since that is the only one currently used. I'll submit pull requests to add additional categories.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

1 participant