kscripting / kscript

Scripting enhancements for Kotlin
MIT License
2.08k stars 126 forks source link

Kscript doesn't work if DependsOn is not on a single line #191

Open PaulWoitaschek opened 6 years ago

PaulWoitaschek commented 6 years ago

The following example - autoformatted by intellij does not work using kscript because the DependsOn block is not parsed correctly. Tested on latest commit c2d1e4707e3a2ced4418facfe4331a77dbe19e53

#!/usr/bin/env kscript
@file:DependsOn(
  "com.beust:jcommander:1.71",
  "org.apache.commons:commons-exec:1.3",
  "com.android.tools:sdk-common:26.3.0-beta02"
)
@file:MavenRepository("google", "https://dl.google.com/dl/android/maven2")

import com.android.ide.common.vectordrawable.Svg2Vector
import com.beust.jcommander.JCommander
import com.beust.jcommander.Parameter
import org.apache.commons.exec.CommandLine
import org.apache.commons.exec.DefaultExecutor
import java.io.File
import java.io.IOException
holgerbrandl commented 6 years ago

Indeed, the currently used regex that pulls out the annotation dies not account for multiline. Would you be willing/feel invited to contribute a PR fixing the regex?

In general we just allowed for several dependencies per annotation because the kotlin jupyter kernel did so via keplin annotations. Personally would prefer a one-annotation-per-dependency approach. Not sure how this is done in the v1.3 scripting API, @ligee? If they just allow for one argument we should at least deprecate more than 1 arguments to ease a future migration path.

holgerbrandl commented 5 years ago

@PaulWoitaschek Do you think you could adjust (via PR) the used regex patterns in kscript to support multiline dependency declaration? It would be a nice enhancement to kscript imho.

thecoden commented 3 years ago

I may attempt to work on this today and pull request if I have time. Worst case, I will provide an updated regex and information here.

holgerbrandl commented 3 years ago

Sounds great to me. Thanks for supporting this project

The Coden notifications@github.com schrieb am Sa., 20. Feb. 2021, 18:54:

I may attempt to work on this today and pull request if I have time. Worst case, I will provide an updated regex and information here.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/holgerbrandl/kscript/issues/191#issuecomment-782723015, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABRB6APB6XVMEVDF35EO7DS77ZPBANCNFSM4GA4A24Q .

thecoden commented 3 years ago

Thank you for the project @holgerbrandl and happy to help where I can.

Upon reviewing the code, I learned that the regex is less an issue than all the annotation matching being based on single lines, so I'll have to rework a bit more of the logic. I will update when I have a little more substance.

thecoden commented 3 years ago

I cloned our fork of the project and added a test with a multiline version of the dependency annotation, but the test succeeds somehow, so I'm looking into the differences between that and running it via an actual script.

holgerbrandl commented 3 years ago

Cool, just let me know once the PR is ready