<legolas_of_mirkw>
<headius> "legolas_of_mirkwood: Hello! Yes,..." <- Thank you! That's more or less what I was writing about in the proposal.
<kares[m]>
Hey, have put up this https://github.com/jruby/jruby/pull/7728 to have a warnings kill switch (setWarningsEnabled) on the Java side working around $VERBOSE
<kares[m]>
There's some refactoring so that warnings go through one method as I was able to add a warnings filter function that way (not part of the PR).
<kares[m]>
Also the incorrect +/-1 line reports are expected to be fixed.
<kares[m]>
For the warnings filter on the Java side I need to do more experiments, whether it's useful to add/have, as that would mean sticking to maintaining IRubyWarnings.ID enum, but with the warnings gem we might be able to achieve the same filtering of the warnings emitted.
<PavanNambi[m]>
* headius: thankyou
<enebo[m]>
headius: a nice stretch goal once it is abstract would be to split when coderange is known to be ascii (like a frozen ascii string) then we could simplify and remove a lot of conditionals
<enebo[m]>
legolas_of_mirkwood: ^
<headius>
Pavan Nambi: No, you certainly wouldn't have to do all of them, but hopefully several
<headius>
enebo: yeah I have concerns about splitting the types up and having too many at a given call site, but I think it's probably typical that you'll only see one or two types at a given location
<headius>
if we see more than two we can just go back to instantiating a general-purpose version that can handle all cases
<headius>
in flight now, looking into this warning thing kares reported
<enebo[m]>
I think part of the motivation is that these happen at dyndispatch sites and likely if you are calling a method on a frozen string which is us-ascii it will never change at that site
<headius>
looks like kares found what I found... we were adding +1 to all warn paths when some pass the real line number
<enebo[m]>
yeah so some places are using parser/build 0-index and others have already added one since then
<enebo[m]>
It would make sense to just make the parser always 1-index and then we would always just pass in real line?
<kares[m]>
yeah and the +1/-1 has been going on at the low level warning while some callers had the line correct while others not - this should now be happening at the source
<kares[m]>
pbly parser and lexer now route through a method that does +1
<kares[m]>
and there's now line adjustment in the warning impl itself
<headius>
sounds right
<enebo[m]>
I have been spiking YARP parser which is pushing a lot of changes through IRBuilder since we effectively need 2 builders now. I could look into correcting line numbers to be 1-indexed
<headius>
the ones coming from actual stack trace have the +1 already so it makes sense to have the parser add 1 before calling warnings
<enebo[m]>
but I can do that later since you (kares) seems to have got things consistent
<kares[m]>
there are existing test for the line numbers I just added one extra test for those that were off
<kares[m]>
the PR has more changes since as noted I started looking into this with the motivation of adding a filtering function ...
<kares[m]>
so I did stuff like review warnings to have a proper ID
<headius>
we have been moving away from maintaining that ID enum
<enebo[m]>
yeah the long term goal is to remove it just because we do not use it
<kares[m]>
yeah I thought so ... that's why the I rather postponed adding the filtering function
<enebo[m]>
The idea of it came with IDE support which we later ripped out to jruby-parser
<enebo[m]>
but once you have signatures they get hard to remove :)
<kares[m]>
okay, yeah will keep that in mind - will need to look into the warning gem, hopefully it can handle most use-cases I had in mind
<enebo[m]>
kares: if there is a need for ID then I guess it is fine to keep them but we need to change all those MISCELLANEOUS ones to real ones
<enebo[m]>
so if filtering would be better served by them that is ok
<headius>
kares: I was looking into these unused block warnings too... many of them are gone in 3.x line that String#lines one
<headius>
like not line
<kares[m]>
oh really? short lives test there ...
<kares[m]>
I can try coming up with a better test - checking the script with MRI as well
<kares[m]>
there were a lot of warnings that were off by 1 I just picked one as a representative in the report
<headius>
yeah I don't know about the others, I just looked at lines
<headius>
PR looks good though
<kares[m]>
will come up with a better test and merge than ... somewhere later this week or next
<kares[m]>
realized I tagged this with 9.3 but I had no intent of fixing this on 9.3 ....
<headius>
hah
<headius>
yeah I am feeling pretty good about walking away from 9.3 at this point