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

Bump jline3Version in order to avoid stackoverflow error #1313

Merged
merged 2 commits into from
Jan 31, 2021

Conversation

rupert-madden-abbott
Copy link
Contributor

Using picocli-shell-jline3:4.6.1, and copying the example containing this README exactly leads to a stack overflow error. Somebody else reported this issue to jline where they have recommended bumping the jline version to 3.18.0.

I've tested locally, encountered the stack overflow, then bumped the version and confirmed the stack overflow is fixed.

@codecov
Copy link

codecov bot commented Jan 23, 2021

Codecov Report

Merging #1313 (e4e3934) into master (84013e3) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #1313   +/-   ##
=========================================
  Coverage     93.78%   93.78%           
  Complexity      469      469           
=========================================
  Files             2        2           
  Lines          6951     6951           
  Branches       1864     1864           
=========================================
  Hits           6519     6519           
  Misses          146      146           
  Partials        286      286           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 84013e3...e4e3934. Read the comment docs.

@remkop
Copy link
Owner

remkop commented Jan 24, 2021

@rupert-madden-abbott thank you for the pull request!
This looks like a good change, and I am happy to merge it, but should we also update the JLine version in the README?

@remkop remkop added type: dependency 📚 theme: shell An issue or change related to interactive (JLine) applications labels Jan 24, 2021
@remkop remkop added this to the 4.7 milestone Jan 24, 2021
@rupert-madden-abbott
Copy link
Contributor Author

rupert-madden-abbott commented Jan 24, 2021

@remkop Thanks very much.

I've had a look at the README and, personally, I don't think maintaining a version compatibility range in a README is a good idea unless you have tests (automated or otherwise) which are specifically testing picocli-shell-jline3 versions against different jline versions.

I'm not sure when this stackoverflow issue was introduced. It may have only been in jline 3.17.1. In that case, the latest version of picocli-shell-jline3 may be compatible with both 3.17.0 (and less) and 3.18.0 (and more). But I haven't tested those versions so I can't say for sure.

All I know is that version 3.17.1 should not be used. If you are happy that the version compatibility table is otherwise correct, perhaps we could just add a note saying to not use jline 3.17.1 with a link to this PR or to the jline issue?

@remkop
Copy link
Owner

remkop commented Jan 24, 2021

I like your suggestion.
Let’s do that.

@rupert-madden-abbott
Copy link
Contributor Author

@remkop awesome, I've pushed an update. See you what you think.

@remkop remkop merged commit b74dae5 into remkop:master Jan 31, 2021
remkop added a commit that referenced this pull request Jan 31, 2021
@remkop
Copy link
Owner

remkop commented Jan 31, 2021

Merged. Thank you for the contribution!

@remkop remkop modified the milestones: 4.7, 4.6.2 Feb 23, 2021
MarkoMackic pushed a commit to MarkoMackic/picocli that referenced this pull request Oct 17, 2021
MarkoMackic added a commit to MarkoMackic/picocli that referenced this pull request Oct 17, 2021
MarkoMackic added a commit to MarkoMackic/picocli that referenced this pull request Oct 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
theme: shell An issue or change related to interactive (JLine) applications type: dependency 📚
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants