Support jline.jansi alongside fusesource.jansi#2503
Support jline.jansi alongside fusesource.jansi#2503Oondanomala wants to merge 4 commits intoremkop:mainfrom
Conversation
|
Thank you! |
…s things This was added in Jansi 2.1.0, which is hopefully old enough (6 years ago).
|
Turns out calling the
|
|
Good news; JLine has now been modularized, so any native allow overrides needed to be passed to the JVM can now be more granular. But it looks like one may not need the native portions of JLine even. Please read this excellent summary covering how JLine can be used and whether native is even required (it probably isn't). @Oondanomala , you might want to check that summary to make sure the solution you're putting in is compatible with its recommendations. |
Turns out the tests depend on Jansi 1.15 to keep Java 5 support, so this is in fact wanted
|
picocli only uses (reflectively, so there is no dependency) Jansi to check whether it has been installed so it can enable ansi on Windows, so it specifically "needs" (it will work fine without it, as it does not attempt to install it itself) the native module and only that one. It has its own code for making ansi codes. And while modern Windows terminals support ansi without needing to install Jansi, considering picocli still supports Java 5 I would say it's best to keep supporting old terminals :) That said, I noticed the test suite actually depends on an even older version of Jansi before |
Jansi has been merged into JLine, which changes the package name (and the system property name).
This pr updates the code to use the updated package name.
Support for the old name is kept to avoid breaking changes. (though I can remove it if it's not wanted)
I chose to prefer the older one if both versions are available to avoid potential breakages, but I can change that if choosing a more up to date version is preferred.
Note: theoutfield isprivatein the new versions,so I switched to the method with the same name (which has always been there so there is no compatibility concern)so I switched to callingisInstalledinstead (which should be more correct anyway). This means Jansi versions older than 2.1.0 will not work. Hopefully that's not a problem.To support both old and new versions of Jansi, it will first try to call
isInstalled(which should be more correct anyway) and only fall back to the previous method ifisInstalledis not present.Fixes #2437