-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
REPL: JLine: follow recommendation to use JNI, not JNA; also JLine 3.27.1 (was 3.27.0) #22205
Conversation
I am not a good person to test this on Windows, as I don't currently even have a working Windows setup... and even if I did, I know nothing about cygwin/conemu/mingw/???, it's just meaningless noises to me I've added the "backport:nominated" label, but it probably shouldn't be backported to LTS too soon? Might be better to ship it in a 3.6.x release first and make sure the Windows users don't take up torches and pitchforks in response? We are more or less YOLOing this in both sbt and Scala 2.13.16, so we might get problem reports from either of those, too. cc @Friendseeker Regardless, I don't think we need to be too frightened, as JLine is in wide use outside of Scala as well. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These changes look good to me.
as per the https://github.com/jline/jline3 readme
and as per discussion and linked items on #22201
fixes #22201
note that as far as I can tell, the stuff I removed from libexec/common-shared is dead code
@philwalk dunno if you're still around but judging from #12405 you might be a good reviewer here
note that I believe we don't need to also port scala/scala#10889 here, since we are already using separate JLine JARs rather than the all-in-one JAR
I've chosen not to upgrade all the way to JLine 3.28.0 at the moment, as it is quite new (2 days ago) and doesn't appear to have any fixes that might be critical.