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

Checksum example: add Kotlin source code on second tab #1201

Merged
merged 1 commit into from
Oct 4, 2020
Merged

Checksum example: add Kotlin source code on second tab #1201

merged 1 commit into from
Oct 4, 2020

Conversation

deining
Copy link
Contributor

@deining deining commented Oct 3, 2020

This PR adds a second tab to the checksum souce code, showing the Kotlin version of the app.
I hope you like the idea (as I do).

This is work in progress, and I'm willing to push this a bit further. Prior to that, I would like your info/opinion on the following topics:

  • I realized that current coderay source highlighter does not support kotlin yet. Source highlighter highlight-js supports much more languages. I see that highlight-js is mentioned in the header of index.adoc, it is commented out, however. Did you experiment with highlight-js already? What were you experiences? Why did you abandon it?

  • With source codes available in multiple languages, we have to keep the description of the code (especially in the source code) as generic as possible. I had a quick look on the current descrption and had the impression that providing one single description for all languages should be doable. What do yo think?

  • The concept of multiple tabs opens up many possibilites, and we have to restrict ourselves a bit, I guess.

    • Languages (Kotlin, Scala, Groovy): what languages would you like to see added here and how would you rank the importance?
    • JDoodle: would you like to see online execution for java only or should we add instances for other languages, too? JDoodle supports Kotlin, Scala and Groovy, so this should be doable.

I'm curious what you think here!

@codecov-commenter
Copy link

codecov-commenter commented Oct 3, 2020

Codecov Report

Merging #1201 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #1201   +/-   ##
=========================================
  Coverage     94.30%   94.30%           
  Complexity      455      455           
=========================================
  Files             2        2           
  Lines          6676     6676           
  Branches       1795     1795           
=========================================
  Hits           6296     6296           
  Misses          102      102           
  Partials        278      278           

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 c2f4b70...3d8d3c6. Read the comment docs.

Copy link
Owner

@remkop remkop left a comment

Choose a reason for hiding this comment

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

Overall, I like this idea!
I had a minor comment on the positional parameter: it should not have a default value because we want it to be a required parameter.

docs/index.adoc Outdated Show resolved Hide resolved
@deining deining requested a review from remkop October 4, 2020 07:58
@remkop remkop merged commit 2453cb4 into remkop:master Oct 4, 2020
@remkop
Copy link
Owner

remkop commented Oct 4, 2020

I merged the change but I get this error when running gradlew ascii:

> Task :asciidoctor
Converting C:\Users\remko\IdeaProjects\picocli3\docs\A-Whirlwind-Tour-of-Picocli.adoc
Exception in thread "main" org.asciidoctor.gradle.backported.AsciidoctorRemoteExecutionException: Error running Asciidoctor whilst attempting to process C:\Users\remko\IdeaProjects\picocli3\docs\A-Whirlwind-Tou
r-of-Picocli.adoc using backend html5
        at org.asciidoctor.gradle.backported.AsciidoctorJavaExec$_convertFiles_closure3.doCall(AsciidoctorJavaExec.groovy:63)
        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.lang.reflect.Method.invoke(Method.java:498)
        at org.codehaus.groovy.reflection.CachedMethod.invoke(CachedMethod.java:98)
        at groovy.lang.MetaMethod.doMethodInvoke(MetaMethod.java:325)
        at org.codehaus.groovy.runtime.metaclass.ClosureMetaClass.invokeMethod(ClosureMetaClass.java:264)
        at groovy.lang.MetaClassImpl.invokeMethod(MetaClassImpl.java:1034)
        at groovy.lang.Closure.call(Closure.java:418)
        at groovy.lang.Closure.call(Closure.java:434)
        at org.codehaus.groovy.runtime.DefaultGroovyMethods.each(DefaultGroovyMethods.java:2125)
        at org.codehaus.groovy.runtime.DefaultGroovyMethods.each(DefaultGroovyMethods.java:2110)
        at org.codehaus.groovy.runtime.DefaultGroovyMethods.each(DefaultGroovyMethods.java:2163)
        at org.asciidoctor.gradle.backported.AsciidoctorJavaExec.convertFiles(AsciidoctorJavaExec.groovy:56)
        at org.asciidoctor.gradle.backported.AsciidoctorJavaExec.access$0(AsciidoctorJavaExec.groovy)
        at org.asciidoctor.gradle.backported.AsciidoctorJavaExec$_run_closure2.doCall(AsciidoctorJavaExec.groovy:50)
        at org.asciidoctor.gradle.backported.AsciidoctorJavaExec$_run_closure2.call(AsciidoctorJavaExec.groovy)
        at org.codehaus.groovy.runtime.DefaultGroovyMethods.each(DefaultGroovyMethods.java:2125)
        at org.codehaus.groovy.runtime.DefaultGroovyMethods.each(DefaultGroovyMethods.java:2110)
        at org.codehaus.groovy.runtime.DefaultGroovyMethods.each(DefaultGroovyMethods.java:2151)
        at org.asciidoctor.gradle.backported.AsciidoctorJavaExec.run(AsciidoctorJavaExec.groovy:48)
        at org.asciidoctor.gradle.backported.AsciidoctorJavaExec.main(AsciidoctorJavaExec.groovy:194)
Caused by: java.lang.NoClassDefFoundError: org/gradle/internal/classpath/Instrumented
        at com.bmuschko.asciidoctorj.tabbedcode.FileUtils.<clinit>(FileUtils.java:9)
        at com.bmuschko.asciidoctorj.tabbedcode.TabbedCodeBlockDocinfoProcessor.process(TabbedCodeBlockDocinfoProcessor.java:23)
        at org.asciidoctor.jruby.extension.processorproxies.DocinfoProcessorProxy.process(DocinfoProcessorProxy.java:97)
        at org.asciidoctor.jruby.extension.processorproxies.DocinfoProcessorProxy$INVOKER$i$1$0$process.call(DocinfoProcessorProxy$INVOKER$i$1$0$process.gen)
        at org.jruby.internal.runtime.methods.JavaMethod$JavaMethodOneOrN.call(JavaMethod.java:958)
        at org.jruby.RubyMethod.call(RubyMethod.java:119)
        at org.jruby.RubyMethod$INVOKER$i$call.call(RubyMethod$INVOKER$i$call.gen)
        at org.jruby.internal.runtime.methods.JavaMethod$JavaMethodZeroOrOneOrNBlock.call(JavaMethod.java:349)
        at org.jruby.runtime.callsite.CachingCallSite.cacheAndCall(CachingCallSite.java:375)
        at org.jruby.runtime.callsite.CachingCallSite.call(CachingCallSite.java:174)
        at org.jruby.ir.interpreter.InterpreterEngine.processCall(InterpreterEngine.java:316)
        at org.jruby.ir.interpreter.StartupInterpreterEngine.interpret(StartupInterpreterEngine.java:72)
        at org.jruby.ir.interpreter.Interpreter.INTERPRET_BLOCK(Interpreter.java:116)
        at org.jruby.runtime.MixedModeIRBlockBody.commonYieldPath(MixedModeIRBlockBody.java:137)
        at org.jruby.runtime.IRBlockBody.doYield(IRBlockBody.java:166)
        at org.jruby.runtime.BlockBody.yield(BlockBody.java:108)
        at org.jruby.runtime.Block.yield(Block.java:184)
        at org.jruby.RubyArray.collect(RubyArray.java:2563)
        at org.jruby.RubyArray.map19(RubyArray.java:2577)
        at org.jruby.RubyArray$INVOKER$i$0$0$map19.call(RubyArray$INVOKER$i$0$0$map19.gen)
        at org.jruby.internal.runtime.methods.JavaMethod$JavaMethodZeroBlock.call(JavaMethod.java:555)
        at org.jruby.runtime.callsite.CachingCallSite.cacheAndCall(CachingCallSite.java:332)
        at org.jruby.runtime.callsite.CachingCallSite.call(CachingCallSite.java:86)
        at org.jruby.runtime.callsite.CachingCallSite.callIter(CachingCallSite.java:93)
        at org.jruby.ir.instructions.CallBase.interpret(CallBase.java:546)
        at org.jruby.ir.interpreter.InterpreterEngine.processCall(InterpreterEngine.java:361)
        at org.jruby.ir.interpreter.StartupInterpreterEngine.interpret(StartupInterpreterEngine.java:72)
        at org.jruby.ir.interpreter.InterpreterEngine.interpret(InterpreterEngine.java:80)
        at org.jruby.internal.runtime.methods.MixedModeIRMethod.INTERPRET_METHOD(MixedModeIRMethod.java:121)
        at org.jruby.internal.runtime.methods.MixedModeIRMethod.call(MixedModeIRMethod.java:108)
        at org.jruby.internal.runtime.methods.DynamicMethod.call(DynamicMethod.java:192)
        at org.jruby.runtime.callsite.CachingCallSite.cacheAndCall(CachingCallSite.java:354)
        at org.jruby.runtime.callsite.CachingCallSite.call(CachingCallSite.java:143)
        at org.jruby.ir.interpreter.InterpreterEngine.processCall(InterpreterEngine.java:345)
        at org.jruby.ir.interpreter.StartupInterpreterEngine.interpret(StartupInterpreterEngine.java:72)
        at org.jruby.ir.interpreter.InterpreterEngine.interpret(InterpreterEngine.java:86)
        at org.jruby.internal.runtime.methods.MixedModeIRMethod.INTERPRET_METHOD(MixedModeIRMethod.java:156)
        at org.jruby.internal.runtime.methods.MixedModeIRMethod.call(MixedModeIRMethod.java:143)
        at org.jruby.internal.runtime.methods.DynamicMethod.call(DynamicMethod.java:200)
        at org.jruby.runtime.callsite.CachingCallSite.cacheAndCall(CachingCallSite.java:375)
        at org.jruby.runtime.callsite.CachingCallSite.call(CachingCallSite.java:174)
        at org.jruby.ir.interpreter.InterpreterEngine.processCall(InterpreterEngine.java:316)
        at org.jruby.ir.interpreter.StartupInterpreterEngine.interpret(StartupInterpreterEngine.java:72)
        at org.jruby.ir.interpreter.InterpreterEngine.interpret(InterpreterEngine.java:92)
        at org.jruby.internal.runtime.methods.MixedModeIRMethod.INTERPRET_METHOD(MixedModeIRMethod.java:191)
        at org.jruby.internal.runtime.methods.MixedModeIRMethod.call(MixedModeIRMethod.java:178)
        at org.jruby.internal.runtime.methods.DynamicMethod.call(DynamicMethod.java:208)
        at org.jruby.runtime.callsite.CachingCallSite.cacheAndCall(CachingCallSite.java:396)
        at org.jruby.runtime.callsite.CachingCallSite.call(CachingCallSite.java:205)
        at org.jruby.ir.interpreter.InterpreterEngine.processCall(InterpreterEngine.java:325)
        at org.jruby.ir.interpreter.StartupInterpreterEngine.interpret(StartupInterpreterEngine.java:72)
        at org.jruby.ir.interpreter.InterpreterEngine.interpret(InterpreterEngine.java:86)
        at org.jruby.internal.runtime.methods.MixedModeIRMethod.INTERPRET_METHOD(MixedModeIRMethod.java:156)
        at org.jruby.internal.runtime.methods.MixedModeIRMethod.call(MixedModeIRMethod.java:143)
        at org.jruby.internal.runtime.methods.DynamicMethod.call(DynamicMethod.java:200)
        at org.jruby.runtime.callsite.CachingCallSite.cacheAndCall(CachingCallSite.java:375)
        at org.jruby.runtime.callsite.CachingCallSite.call(CachingCallSite.java:174)
        at org.jruby.ir.interpreter.InterpreterEngine.processCall(InterpreterEngine.java:316)
        at org.jruby.ir.interpreter.StartupInterpreterEngine.interpret(StartupInterpreterEngine.java:72)
        at org.jruby.ir.interpreter.InterpreterEngine.interpret(InterpreterEngine.java:92)
        at org.jruby.internal.runtime.methods.MixedModeIRMethod.INTERPRET_METHOD(MixedModeIRMethod.java:191)
        at org.jruby.internal.runtime.methods.MixedModeIRMethod.call(MixedModeIRMethod.java:178)
        at org.jruby.internal.runtime.methods.DynamicMethod.call(DynamicMethod.java:208)
        at org.jruby.runtime.callsite.CachingCallSite.cacheAndCall(CachingCallSite.java:396)
        at org.jruby.runtime.callsite.CachingCallSite.call(CachingCallSite.java:205)
        at org.jruby.ir.interpreter.InterpreterEngine.processCall(InterpreterEngine.java:325)
        at org.jruby.ir.interpreter.StartupInterpreterEngine.interpret(StartupInterpreterEngine.java:72)
        at org.jruby.ir.interpreter.Interpreter.INTERPRET_BLOCK(Interpreter.java:116)
        at org.jruby.runtime.MixedModeIRBlockBody.commonYieldPath(MixedModeIRBlockBody.java:137)
        at org.jruby.runtime.IRBlockBody.doYield(IRBlockBody.java:166)
        at org.jruby.runtime.BlockBody.yield(BlockBody.java:108)
        at org.jruby.runtime.Block.yield(Block.java:184)
        at org.jruby.RubyIO.ensureYieldClose(RubyIO.java:1164)
        at org.jruby.RubyIO.open(RubyIO.java:1158)
        at org.jruby.RubyIO$INVOKER$s$0$0$open.call(RubyIO$INVOKER$s$0$0$open.gen)
        at org.jruby.runtime.callsite.CachingCallSite.cacheAndCall(CachingCallSite.java:332)
        at org.jruby.runtime.callsite.CachingCallSite.call(CachingCallSite.java:86)
        at org.jruby.runtime.callsite.CachingCallSite.callIter(CachingCallSite.java:93)
        at org.jruby.ir.instructions.CallBase.interpret(CallBase.java:546)
        at org.jruby.ir.interpreter.InterpreterEngine.processCall(InterpreterEngine.java:361)
        at org.jruby.ir.interpreter.StartupInterpreterEngine.interpret(StartupInterpreterEngine.java:72)
        at org.jruby.internal.runtime.methods.MixedModeIRMethod.INTERPRET_METHOD(MixedModeIRMethod.java:86)
        at org.jruby.internal.runtime.methods.MixedModeIRMethod.call(MixedModeIRMethod.java:73)
        at org.jruby.internal.runtime.methods.DynamicMethod.call(DynamicMethod.java:174)
        at org.jruby.RubyClass.finvoke(RubyClass.java:812)
        at org.jruby.runtime.Helpers.invoke(Helpers.java:464)
        at org.jruby.RubyBasicObject.callMethod(RubyBasicObject.java:376)
        at org.asciidoctor.jruby.internal.JRubyAsciidoctor.convertFile(JRubyAsciidoctor.java:390)
        at org.asciidoctor.jruby.internal.JRubyAsciidoctor.convertFile(JRubyAsciidoctor.java:366)
        at org.asciidoctor.gradle.backported.AsciidoctorJavaExec$_convertFiles_closure3.doCall(AsciidoctorJavaExec.groovy:61)
        ... 22 more
Caused by: java.lang.ClassNotFoundException: org.gradle.internal.classpath.Instrumented
        at java.net.URLClassLoader.findClass(URLClassLoader.java:382)
        at java.lang.ClassLoader.loadClass(ClassLoader.java:424)
        at sun.misc.Launcher$AppClassLoader.loadClass(Launcher.java:349)
        at java.lang.ClassLoader.loadClass(ClassLoader.java:357)
        ... 112 more

> Task :asciidoctor FAILED

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':asciidoctor'.
> Process 'command 'C:\apps\jdk1.8.0_202\bin\java.exe'' finished with non-zero exit value 1

@deining
Copy link
Contributor Author

deining commented Oct 4, 2020

I merged the change but I get this error when running gradlew ascii:

This might be an gradle issue (#14727).
Are you using a gradle version > 6.5?

In general, gradlew ascii works and worked for me. But yes, I saw the same error once on my machine, too. Maybe since I used a different gradle version (I'm not sure, though).

@remkop
Copy link
Owner

remkop commented Oct 4, 2020

Yes, we are using Gradle 6.6:
https://github.com/remkop/picocli/blob/master/gradle/wrapper/gradle-wrapper.properties

The upgrade was needed to build on Java 15... (gradle/gradle#13628)

If there is no workaround we may need to backtrack and wait until gradle/gradle#14727 is fixed.

@deining
Copy link
Contributor Author

deining commented Oct 4, 2020

This might be an gradle issue (#14727).

No, this is not an gradle issue.
With PR #1202 applied, gradle ascii can be run successfully again.

@remkop
Copy link
Owner

remkop commented Oct 4, 2020


Completely different topic: I noticed that your contributions are not counted correctly here. Could it be that the email address that you used to commit is not registered in GitHub? I suspect that if you add that email address, the stats will show up correctly.


remkop added a commit that referenced this pull request Oct 6, 2020
@remkop remkop added this to the 4.6 milestone Oct 6, 2020
@remkop
Copy link
Owner

remkop commented Oct 6, 2020

I just noticed your questions (sorry, bit hectic here)...

Did you experiment with highlight-js already? What were you experiences? Why did you abandon it?

This is a long time ago and I honestly don't remember. I should have made notes. I think it was because I was also experimenting with dark themes and those did not work together but not sure anymore. (About dark themes, I decided against it because I felt it takes away (lessens the "wow" factor) from the screenshots showing picocli's use of ANSI colors.)

With source codes available in multiple languages, we have to keep the description of the code (especially in the source code) as generic as possible. I had a quick look on the current description and had the impression that providing one single description for all languages should be doable. What do yo think?

Not sure what you mean with this. Are some descriptions very language-specific? Should be fine to change in general I think. One exception may be the Picocli in Other Languages section, where I think we should not used tabbed examples with multiple languages: better to keep each subsection focused on its own language.

Languages (Kotlin, Scala, Groovy): what languages would you like to see added here and how would you rank the importance?

For the first (checksum) example it would certainly be great to have all four examples. I would sort the tabs Java Groovy Kotlin Scala - Java as the primary example, and the others in alphabetic order.
For the other examples it would be great if they could all be in all four languages, but this may take some time. We could ask for community contributions on Twitter or other social media.

JDoodle: would you like to see online execution for java only or should we add instances for other languages, too? JDoodle supports Kotlin, Scala and Groovy, so this should be doable.

Hm, not sure. We would have to experiment a bit. I worry a bit that having four links under each example may be distracting for readers. Also, it may become a maintenance burden: if we need to change an example we would need to change the asciidoc, but also four different files on JDoodle. I would not go overboard with JDoodle for now. If we do anything, perhaps just a few strategic examples at most.

@deining
Copy link
Contributor Author

deining commented Oct 6, 2020

Completely different topic: I noticed that your contributions are not counted correctly here. Could it be that the email address that you used to commit is not registered in GitHub? I suspect that if you add that email address, the stats will show up correctly.

Thanks for poiting that out!
I added email addresses to my GitHub account, and the stats look much better for me now :-)

@deining
Copy link
Contributor Author

deining commented Oct 6, 2020

For the first (checksum) example it would certainly be great to have all four examples.

Absolutely. Stay tuned, I will author a PR for this soon.

For the other examples it would be great if they could all be in all four languages,

I agree. Let's see what I can do here. As said, it may take some time, but I'm willing to work on this.

About maintenance burden:
I realized that within any .adoc file, you can include from a URI which opens up the possibility to include a source code file directly from picocli GitHub repo. Not sure whether this is a good idea, though.

@deining deining deleted the checksum-tabbed-code branch October 6, 2020 17:44
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