Docu, new code samples: Kotlin versions, minor changes #1296

deining · 2021-01-04T14:49:13Z

This PR adds Kotlin versions for the new code samples in picocli 4.6 (IModelTransformer, IParameterProcessor).
While working on the IParameterProcessor sample, I made minor changes to the code sample and to the documentation in order to improve readability and intelligibility. I hope you like them.
Additionally, I added a third column to the comparison table PicocliBaseScript2 vs. PicocliBaseScript.

- remove incorrect access modifier - add Kotlin version

codecov-io · 2021-01-04T15:39:38Z

Codecov Report

Merging #1296 (c0bd988) into master (97c73b1) will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master    #1296    /-   ##
=========================================
  Coverage     93.78%   93.78%           
  Complexity      469      469           
=========================================
  Files             2        2           
  Lines          6948     6948           
  Branches       1864     1864           
=========================================
  Hits           6516     6516           
  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 97c73b1...c0bd988. Read the comment docs.

remkop

Wow, awesome work again!

The only think I would like to change is that I think we should keep Editor.defaultEditor to represent the absence of choice by the end user, rather than just picking one of the actual editors as the default. I put more detailed comments with the proposed changes.

Other than that, it looks great!

docs/index.adoc

remkop · 2021-01-04T23:30:28Z

docs/index.adoc

           "Example: edit --open=idea FILE opens IntelliJ IDEA (notice the '=' separator)",
-           "         edit --open FILE opens the specified file in the default editor"
+           "         edit --open FILE opens the specified file in eclipse"
       })


Similar to the above, I think it is meaningful to have a defaultEditor choice, so let's keep the original description.

docs/index.adoc

remkop · 2021-01-04T23:36:36Z

docs/index.adoc

-               // act as if the user specified --open=defaultEditor
-               args.push(Editor.defaultEditor.name());
+               // act as if the user specified --open=eclipse
+               args.push(editor.name());


I think it is good to explicitly use the enum constant that represents the default, so Editor.defaultEditor (or Editor.eclipse if Eclipse were the default).

This is mostly for clarity: so that people reading this code example in the documentation would not have to refer back in the code to what the actual value would be.

I realize that in "real" code there is a tension here with the principle of having information in only once place. To be honest, I am not sure what I would do in a "real" application in this case. :-)

But for documentation purposes, I believe that duplicating the default value in the preprocessor is better because it requires less work on the part of the reader.

docs/index.adoc

deining · 2021-01-05T07:19:02Z

Wow, awesome work again!

Great you like it!
I realized that the comparison table PicocliBaseScript2 vs. PicocliBaseScript is present in the release notes, too. For the sake of consistency, I would like to modify this table, too. I hope this is fine for you.

The only think I would like to change is that I think we should keep Editor.defaultEditor to represent the absence of choice by the end user, rather than just picking one of the actual editors as the default. I put more detailed comments with the proposed changes.

I will work on this shortly, you may expect an updated PR soon.

Other than that, it looks great!

Thank you!

While working on this, two further issues came up that I would like to raise:

I'm wondering, whether the API Docs for the package picocli.groovy are up to date. I would suspect these docs at this URL, but these documents are 4.0.0-alpha-3. I didn't find a link to these API docs in any README.mdfile in the repository either, I think this is something that should be added.
ParameterConsumer vs IParameterPreprocessor: AFAICS, the IParameterPreprocessor is superior to the ParameterConsumer in all aspects. Is there any reason to still stick to the ParameterConsumer? If not, this interface should be marked as deprecated IMHO. Also, chapter 11.15. Custom Parameter Processing lists both plugin in chronological order concerning their authoring date. I think they should be ordered according to relevance for the user, therefore I would suggest to swap the first two subchapters.

remkop · 2021-01-05T07:44:38Z

I realized that the comparison table PicocliBaseScript2 vs. PicocliBaseScript is present in the release notes, too. For the sake of consistency, I would like to modify this table, too. I hope this is fine for you.

I'm okay to leave the release notes as they are, and just evolve the user manual. I see the release notes as more of a snapshot of how things were at some point in time, so there is some value in not updating them.

I'm wondering, whether the API Docs for the package picocli.groovy are up to date. I would suspect these docs at this URL, but these documents are 4.0.0-alpha-3. I didn't find a link to these API docs in any README.mdfile in the repository either, I think this is something that should be added.

Oh, good catch! I had completely forgotten about this...
The Groovy classes were previously in the main picocli jar, now they are in a separate jar... I suspect these javadoc pages are still there from when these classes were in the picocli jar.

It makes sense to publish Javadoc for the other jars, not just for the picocli jar, but also for picocli-groovy, picocli-jline2/3, picocli-spring-boot and picocli-codegen. The best way to deal with this is probably to update the copyDocs task in build.gradle. Also, like you said, we need to link to their top-level pages from the user manual and project README.

IParameterConsumer vs IParameterPreprocessor: AFAICS, the IParameterPreprocessor is superior to the IParameterConsumer in all aspects. Is there any reason to still stick to the IParameterConsumer? If not, this interface should be marked as deprecated IMHO. Also, chapter 11.15. Custom Parameter Processing lists both plugin in chronological order concerning their authoring date. I think they should be ordered according to relevance for the user, therefore I would suggest to swap the first two subchapters.

Swapping these in the manual makes sense; IParameterPreprocessor is more flexible than IParameterConsumer. That said, IParameterConsumer is simpler, so I can imagine people preferring to use that one. I am not in a rush to deprecate IParameterConsumer. It is probably a good idea to update the javadoc for IParameterConsumer to mention that IParameterPreprocessor is a more powerful and flexible alternative.

deining · 2021-01-05T08:07:42Z

we need to link to their top-level pages from the user manual and project README.

As an addition, we could develop/author an overview page that contains links to the apidocs of all subprojects. Once we made this overview page avaible at the subdomain apidocs.picocli.info, this URL could serve as single starting point for accessing all apidocs (just a proposal/ an idea).

remkop · 2021-01-05T08:10:30Z

I created #1298 and put some ideas on how to deal with the Javadoc for the other modules there, let's discuss further on that ticket.

deining · 2021-01-05T10:53:37Z

I resolved all issues from your review meanwhile.
Thanks for considering my PR.

remkop · 2021-01-05T12:20:09Z

Merged and published the HTML. Many thanks!

…ovements

…her improvements" This reverts commit 4183b20.

This reverts commit da13d9a.

…her improvements" This reverts commit 4183b20.

This reverts commit da13d9a.

deining added 3 commits January 4, 2021 12:08

Amend documentation on IParameterPreprocessor Parser Plugin

42c0527

Code sample IParameterPreprocessor: add Kotlin version

ff05cee

Docu: IModelTransformer, code sample

b0c922e

- remove incorrect access modifier - add Kotlin version

Docu: improve readability of comparison table for groovy base scripts

4737b8a

remkop requested changes Jan 4, 2021

View reviewed changes

remkop added the type: doc 📘 label Jan 5, 2021

remkop mentioned this pull request Jan 5, 2021

Publish javadoc for other picocli modules #1298

Closed

Incorporate feedback from maintainer (#1296)

c0bd988

deining requested a review from remkop January 5, 2021 10:52

remkop approved these changes Jan 5, 2021

View reviewed changes

remkop merged commit 286b756 into remkop:master Jan 5, 2021

deining deleted the kotlin_samples branch January 5, 2021 12:10

remkop added this to the 4.7 milestone Jan 5, 2021

remkop added a commit that referenced this pull request Jan 5, 2021

[#1296] DOC: add Kotlin code samples to user manual; other improvements

7fc413d

deining mentioned this pull request Jan 5, 2021

Extend javadoc of IParameterConsumer interface #1299

Merged

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

Incorporate feedback from maintainer (remkop#1296)

da13d9a

MarkoMackic pushed a commit to MarkoMackic/picocli that referenced this pull request Oct 17, 2021

[remkop#1296] DOC: add Kotlin code samples to user manual; other impr…

4183b20

…ovements

MarkoMackic added a commit to MarkoMackic/picocli that referenced this pull request Oct 17, 2021

Revert "[remkop#1296] DOC: add Kotlin code samples to user manual; ot…

b60ed9b

…her improvements" This reverts commit 4183b20.

MarkoMackic added a commit to MarkoMackic/picocli that referenced this pull request Oct 17, 2021

Revert "Incorporate feedback from maintainer (remkop#1296)"

a73d728

This reverts commit da13d9a.

MarkoMackic added a commit to MarkoMackic/picocli that referenced this pull request Oct 17, 2021

Revert "[remkop#1296] DOC: add Kotlin code samples to user manual; ot…

401ee79

…her improvements" This reverts commit 4183b20.

MarkoMackic added a commit to MarkoMackic/picocli that referenced this pull request Oct 17, 2021

Revert "Incorporate feedback from maintainer (remkop#1296)"

94d414d

This reverts commit da13d9a.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Docu, new code samples: Kotlin versions, minor changes #1296

Docu, new code samples: Kotlin versions, minor changes #1296

deining commented Jan 4, 2021 •

edited

Loading

codecov-io commented Jan 4, 2021 •

edited

Loading

remkop left a comment

remkop Jan 4, 2021

remkop Jan 4, 2021

deining commented Jan 5, 2021

remkop commented Jan 5, 2021 •

edited

Loading

deining commented Jan 5, 2021

remkop commented Jan 5, 2021

deining commented Jan 5, 2021

remkop commented Jan 5, 2021

Docu, new code samples: Kotlin versions, minor changes #1296

Docu, new code samples: Kotlin versions, minor changes #1296

Conversation

deining commented Jan 4, 2021 • edited Loading

codecov-io commented Jan 4, 2021 • edited Loading

Codecov Report

remkop left a comment

Choose a reason for hiding this comment

remkop Jan 4, 2021

Choose a reason for hiding this comment

remkop Jan 4, 2021

Choose a reason for hiding this comment

deining commented Jan 5, 2021

remkop commented Jan 5, 2021 • edited Loading

deining commented Jan 5, 2021

remkop commented Jan 5, 2021

deining commented Jan 5, 2021

remkop commented Jan 5, 2021

deining commented Jan 4, 2021 •

edited

Loading

codecov-io commented Jan 4, 2021 •

edited

Loading

remkop commented Jan 5, 2021 •

edited

Loading