Skip to content

Conversation

@S-Gaertner
Copy link
Contributor

  • changed terminal width detection regex pattern for Windows
  • check for process termination in order to fail faster

@codecov-io
Copy link

codecov-io commented Apr 16, 2020

Codecov Report

Merging #993 into master will decrease coverage by 0.03%.
The diff coverage is 50.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #993      +/-   ##
============================================
- Coverage     94.68%   94.65%   -0.04%     
  Complexity      415      415              
============================================
  Files             2        2              
  Lines          6136     6138       +2     
  Branches       1637     1638       +1     
============================================
  Hits           5810     5810              
  Misses           89       89              
- Partials        237      239       +2     
Impacted Files Coverage Δ Complexity Δ
src/main/java/picocli/CommandLine.java 94.50% <50.00%> (-0.04%) 281.00 <0.00> (ø)

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 bb0019f...281156a. Read the comment docs.

if (size.intValue() >= 0) { break; }
try { Thread.sleep(25); } catch (InterruptedException ignored) {}
} while (System.currentTimeMillis() < now + 2000);
} while (System.currentTimeMillis() < now + 2000 && t.isAlive());
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Were you able to test that this prevents the 2 second timeout when the pattern cannot match anything (e.g. with the old pattern on a German language environment)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I manually tested that on my system with the old pattern. The 2 second delay was gone as soon as I introduced the alive test for the thread.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, thank you!

@remkop remkop added type: enhancement ✨ theme: shell An issue or change related to interactive (JLine) applications theme: usagehelp An issue or change related to the usage help message labels Apr 16, 2020
@remkop remkop added this to the 4.3 milestone Apr 16, 2020
@remkop
Copy link
Owner

remkop commented Apr 16, 2020

Overall looks good. I have some other work in progress but I will try to merge this soon.

Thank you for the contribution!

@S-Gaertner
Copy link
Contributor Author

You're welcome! Thank you for this great project :)

@remkop remkop merged commit 279e552 into remkop:master Apr 16, 2020
remkop added a commit that referenced this pull request Apr 16, 2020
@remkop
Copy link
Owner

remkop commented Apr 16, 2020

Can I ask:

  • What is it that you like about picocli?
  • What would you do if picocli did not exist?

@S-Gaertner
Copy link
Contributor Author

What I like about picocli:

  • It allows complex use cases, yet is pretty easy to use - especially with the convenience methods.
  • It has zero dependencies - I am reluctant to include dependencies that themselves entail more dependencies.
  • It has superb documentation.
  • It is open source, mature, active, feature-rich and has a friendly community.

What I would do without picocli:

  • I already have rolled my own command line parser for simple uses cases. Now my use cases grew more complex, so I probably would have adapted it to some degree and also simplified my use cases (and at the same time would provide more user training for the probably more complicated command line invocation).
  • I also would have looked into alternatives like JCommander.

@remkop
Copy link
Owner

remkop commented Apr 16, 2020

@S-Gaertner Great feedback, thank you! 👍

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 theme: usagehelp An issue or change related to the usage help message type: enhancement ✨

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants