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

Quick guide: online execution of sample apps #1196

Merged
merged 1 commit into from
Oct 2, 2020
Merged

Quick guide: online execution of sample apps #1196

merged 1 commit into from
Oct 2, 2020

Conversation

deining
Copy link
Contributor

@deining deining commented Sep 30, 2020

This PR, ready for merging, supersedes #1188.
This is my attempt to add online execution to the quick guide.
Please let me know how you like it.

@codecov-commenter
Copy link

codecov-commenter commented Sep 30, 2020

Codecov Report

Merging #1196 into master will increase coverage by 0.04%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #1196      +/-   ##
============================================
+ Coverage     94.26%   94.30%   +0.04%     
  Complexity      455      455              
============================================
  Files             2        2              
  Lines          6674     6674              
  Branches       1793     1794       +1     
============================================
+ Hits           6291     6294       +3     
+ Misses          105      102       -3     
  Partials        278      278              
Impacted Files Coverage Δ Complexity Δ
src/main/java/picocli/CommandLine.java 94.14% <0.00%> (+0.04%) 314.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 5aeef45...fa6c8da. Read the comment docs.

@remkop
Copy link
Owner

remkop commented Oct 1, 2020

Thanks for the PR.
I saw your PR but haven’t had time to look at it in detail. Probably not until this weekend...

@remkop remkop merged commit 6dc6ed4 into remkop:master Oct 2, 2020
@remkop remkop added this to the 4.6 milestone Oct 2, 2020
@remkop
Copy link
Owner

remkop commented Oct 2, 2020

Thank you very much!
Merged, still polishing a bit before publishing the rendered HTML.

I changed the titles a bit: instead of having to sections called "Online execution", there is now one section "ASCIIArt execution" and one section "ISOCodeResolver execution", similar for "Source code explained".

I also added a brief note above the embedded JDoodle.com section, in case something goes wrong, with a link that people can use to display the example in a separate browser tab.

TOC depth: I changed it back to only one level. I think this is less intimidating. The user manual has a very long TOC that allows more precise navigation, but also emphasizes that the document has a lot of detail. I thought it made sense to try to stay high-level in the Quick Guide.

I made Basic Example and Subcommand Example top-level sections in Quick Guide so they show up separately in the TOC.

The paramLabel in the executable examples suggest that users need to enter two codes:

// AsciiArt
paramLabel = "word1 [word2]" --> should be paramLabel = "<word>"

// ISOCodeResolver country
 paramLabel = "<country code 1> <country code 2>" --> should be paramLabel = "<country code>"

// ISOCodeResolver language
paramLabel = "<code> <code2>" --> should be paramLabel = "<language code>"

The reason is that picocli automatically adds an ellipsis ... behind any parameter (and its param label), when the parameter can be specified multiple times for that option. This is the convention, so it is not necessary to add additional param labels to suggest that multiple parameters can be specified.
Can you update the examples?

remkop added a commit that referenced this pull request Oct 2, 2020
@deining
Copy link
Contributor Author

deining commented Oct 2, 2020

Merged, still polishing a bit before publishing the rendered HTML.

Great work I appreciate it!

Can you update the examples?

Yes. JDoodle examples are updated already. I will correct the samples in the .adoc and example section soon.

I willing to author another pull request adding JDoodle online execution for the checksum example in the user manual.
Would that be in your interest?

@remkop
Copy link
Owner

remkop commented Oct 2, 2020

Yes, that would be great!
For the user manual I prefer to have a link to JDoodle rather than an embedded example, maybe in a TIP admonition?

FYI
I need to work on a picocli presentation for the Japan Java user group next week so I may be slow to respond.

@remkop
Copy link
Owner

remkop commented Oct 3, 2020

I pushed the rendered HTML.
It looks very nice I think! Many thanks!

I thought it would be better to change the order and have the explanation first, and then allow people to play with execution.
That has a nice "flow" to it, I felt. What do you think?

One minor detail, the ISOCodeResolver example has a typo in the paramLabel for language code>, there is a missing < opening fish bracket. Can you take a look?

paramLabel = "language code>",  --> missing `<` opening fish bracket

@deining
Copy link
Contributor Author

deining commented Oct 3, 2020

I pushed the rendered HTML.
It looks very nice I think! Many thanks!

You are welcome.

I thought it would be better to change the order and have the explanation first, and then allow people to play with execution.
That has a nice "flow" to it, I felt. What do you think?

I see the point. Looks good to me.

One minor detail, the ISOCodeResolver example has a typo in the paramLabel for language code>, there is a missing < opening fish bracket. Can you take a look?

This is fixed now.

@remkop
Copy link
Owner

remkop commented Oct 3, 2020

Thank you for the quick fix!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants