jbang icon indicating copy to clipboard operation
jbang copied to clipboard

Introduce `//PROPS` for script properties

Open quintesse opened this issue 2 months ago • 6 comments

Looking at a JBang script someone wrote:

//SOURCES ../jabls/src/main/java/org/jabref/languageserver/BibtexTextDocumentService.java
//SOURCES ../jabls/src/main/java/org/jabref/languageserver/BibtexWorkspaceService.java
//SOURCES ../jabls/src/main/java/org/jabref/languageserver/controller/LanguageServerController.java
//SOURCES ../jabls/src/main/java/org/jabref/languageserver/ExtensionSettings.java
//SOURCES ../jabls/src/main/java/org/jabref/languageserver/LspClientHandler.java
//SOURCES ../jabls/src/main/java/org/jabref/languageserver/LspLauncher.java
//SOURCES ../jabls/src/main/java/org/jabref/languageserver/util/LspConsistencyCheck.java
//SOURCES ../jabls/src/main/java/org/jabref/languageserver/util/LspDiagnosticBuilder.java
//SOURCES ../jabls/src/main/java/org/jabref/languageserver/util/LspDiagnosticHandler.java
//SOURCES ../jabls/src/main/java/org/jabref/languageserver/util/LspIntegrityCheck.java

I thought it would be nice if you could do this:

//PROPS base=../jabls/src/main/java/org/jabref/languageserver
//SOURCES ${base}/BibtexTextDocumentService.java
//SOURCES ${base}/BibtexWorkspaceService.java
//SOURCES ${base}/controller/LanguageServerController.java
//SOURCES ${base}/ExtensionSettings.java
//SOURCES ${base}/LspClientHandler.java
//SOURCES ${base}/LspLauncher.java
//SOURCES ${base}/util/LspConsistencyCheck.java
//SOURCES ${base}/util/LspDiagnosticBuilder.java
//SOURCES ${base}/util/LspDiagnosticHandler.java
//SOURCES ${base}/util/LspIntegrityCheck.java

This properties would only be available during parsing of the script's directives. They would be evaluated in order (so you can have one property based on another). Once evaluated they are "globally" applied (so the place in the file does not affect anything, if there are duplicate property names only the last value will ever be used) There would be no associated command line --props option because we can already set properties with -D. Properties set on the command line override the ones defined with //PROPS

quintesse avatar Sep 25 '25 10:09 quintesse

Related but not.quite https://github.com/jbangdev/jbang/issues/1541

maxandersen avatar Sep 25 '25 15:09 maxandersen

btw. +1 on having this as for the (more common?) usecase of dep versions:

/DEPS io.quarkus:quarkus-resteasy:${quarkus.version:3.25}
//DEPS io.quarkus:quarkus-arc:${quarkus.version:3.25}
//DEPS io.quarkus:quarkus-jdbc-h2:${quarkus.version:3.25}

vs

//PROPS quarkus.version=3.25
/DEPS io.quarkus:quarkus-resteasy:${quarkus.version}
//DEPS io.quarkus:quarkus-arc:${quarkus.version}
//DEPS io.quarkus:quarkus-jdbc-h2:${quarkus.version}

It basically becomes the directive vaiant of -D and could argue for consistency should support --props on command line.

maxandersen avatar Sep 26 '25 07:09 maxandersen

could argue for consistency should support --props on command line.

Normally I'd agree, but if it causes confusion I'd rather break consistency a bit. (Confusion not only for users, but even for us, we'd have two ways to set properties but they'd work just very slightly different (eg. --props would not be set as System properties and would not be passed on to the script that's being run)

quintesse avatar Sep 26 '25 12:09 quintesse

ah, now I see your point. --props aren't runtime/system properties.

But wouldn't that mean exactly we would need a --props to allow precise override/definition ?

or are you saying -D is sufificent as it will influence the //PROPS ?

maxandersen avatar Sep 26 '25 12:09 maxandersen

or are you saying -D is sufificent as it will influence the //PROPS ?

Exactly. I don't think we want two different kind of properties (say ${foo} and %[foo]) so given the fact that both -D and //PROPS would affect the same kind of property (the ${foo} ones) I'd simply go for the simple solution of -D on the command line and //PROPS in the source.

We could of course rename //PROPS to //D to be consistent 😀

But admittedly this (using -D) means that there could be properties that "leak" into the app at runtime that were only meant to be used in the source file. But we already have this issue right now with //DEPS io.quarkus:quarkus-arc:${quarkus.version:3.25}.

quintesse avatar Sep 26 '25 12:09 quintesse

Exactly. I don't think we want two different kind of properties (say ${foo} and %[foo]) so given the fact that both -D and //PROPS would affect the same kind of property (the ${foo} ones) I'd simply go for the simple solution of -D on the command line and //PROPS in the source.

i'm not suggesting that we have different kind of properties, ie. ${foo} and %[foo] - more that -D becomes available during runtime too --props don't.

We could of course rename //PROPS to //D to be consistent 😀

but that would be inconsistent; because they are not the same.

But admittedly this (using -D) means that there could be properties that "leak" into the app at runtime that were only meant to be used in the source file. But we already have this issue right now with //DEPS io.quarkus:quarkus-arc:${quarkus.version:3.25}.

but thats just because its using -Dquarkus.version=3.19 , if it used --props quarkus.version=3.19 it would only apply at build/compile time is my thinking. Maybe do --CDquarkus.version for compile-define property, and --RD for runtime-defined property to match runtime/compile options?

maxandersen avatar Sep 26 '25 18:09 maxandersen