Skip to content

Commit

Permalink
Fix Inconsistency with camelCase and kebab-case config params #4702 (…
Browse files Browse the repository at this point in the history 
…solve #4033)


Signed-off-by: Ben Sherman <[email protected]>
Co-authored-by: Paolo Di Tommaso <[email protected]>
  • Loading branch information
@bentsherman @pditommaso
bentsherman and pditommaso authored Sep 30, 2024
1 parent b5de4fd commit 349f4f6
Show file tree
Hide file tree
Showing 4 changed files with 63 additions and 221 deletions.
14 changes: 14 additions & 0 deletions modules/nextflow/src/main/groovy/nextflow/cli/CmdRun.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ import nextflow.script.ScriptRunner
import nextflow.util.CustomPoolFactory
import nextflow.util.Duration
import nextflow.util.HistoryFile
import org.apache.commons.lang.StringUtils
import org.fusesource.jansi.AnsiConsole
import org.yaml.snakeyaml.Yaml
/**
Expand Down Expand Up @@ -670,6 +671,19 @@ class CmdRun extends CmdBase implements HubOptions {
}
}

static protected void addParam0(Map params, String key, Object value) {
if( key.contains('-') )
key = kebabToCamelCase(key)
params.put(key, value)
}

static protected String kebabToCamelCase(String str) {
final result = new StringBuilder()
str.split('-').eachWithIndex { String entry, int i ->
result << (i>0 ? StringUtils.capitalize(entry) : entry )
}
return result.toString()
}

static protected parseParamValue(String str) {
if ( SysEnv.get('NXF_DISABLE_PARAMS_TYPE_DETECTION') )
Expand Down
165 changes: 17 additions & 148 deletions modules/nextflow/src/main/groovy/nextflow/script/ScriptBinding.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import groovy.util.logging.Slf4j
import nextflow.NF
import nextflow.Session
import nextflow.exception.AbortOperationException
import org.apache.commons.lang.StringUtils
/**
* Defines the script execution context. By default provided the following variables
* <li>{@code __$session}: the current execution session
Expand Down Expand Up @@ -202,23 +201,17 @@ class ScriptBinding extends WorkflowBinding {
}

/**
* Holds parameter immutable values
* Implements immutable params map
*/
@CompileStatic
static class ParamsMap implements Map<String,Object> {

private Set<String> readOnlyNames

private List<String> realNames

private List<String> scriptAssignment = []

@Delegate
private Map<String,Object> target

ParamsMap() {
readOnlyNames = []
realNames = []
target = new LinkedHashMap<>()
}

Expand All @@ -227,23 +220,15 @@ class ScriptBinding extends WorkflowBinding {
putAll(values)
}

private ParamsMap(ParamsMap template, Map overrides) {
this(template)
allowNames(overrides.keySet())
putAll(overrides)
}
private ParamsMap(ParamsMap values, Map<String,?> overrides) {
this(values)

ParamsMap copyWith(Map overrides) {
return new ParamsMap(this, overrides)
for( def entry : overrides )
put0(entry.key, entry.value, true)
}

private ParamsMap allowNames(Set names) {
for( String name : names ) {
final name2 = name.contains('-') ? hyphenToCamelCase(name) : camelCaseToHyphen(name)
readOnlyNames.remove(name)
readOnlyNames.remove(name2)
}
return this
ParamsMap copyWith(Map<String,?> overrides) {
return new ParamsMap(this, overrides)
}

@Override
Expand All @@ -269,142 +254,26 @@ class ScriptBinding extends WorkflowBinding {
@Override
String put(String name, Object value) {
assert name
(name in scriptAssignment
? log.warn("`params.$name` is defined multiple times -- Assignments following the first are ignored")
: scriptAssignment << name )
if( name in scriptAssignment )
log.warn("`params.$name` is defined multiple times -- Assignments following the first are ignored")
else
scriptAssignment << name
return put0(name,value)
}

private String put0(String name, Object value) {

// keep track of the real name
realNames << name

// normalize the name
def name2 = name.contains('-') ? hyphenToCamelCase(name) : camelCaseToHyphen(name)

final readOnly = name in readOnlyNames || name2 in readOnlyNames

def result = null
if( !readOnly ) {
readOnlyNames << name
readOnlyNames << name2
result = target.put(name, value)
target.put(name2, value)
}

return result
private String put0(String name, Object value, boolean allowOverride=false) {
return allowOverride || !target.containsKey(name)
? target.put(name, value)
: null
}

@Override
void putAll(Map other) {
for( Entry<String,Object> entry : other.entrySet() ) {
void putAll(Map<? extends String,? extends Object> other) {
for( def entry : other.entrySet() ) {
put0(entry.getKey(), entry.getValue())
}
}

/**
* Renders a string containing all name-value pairs as a command line string
*
* @param opts String formatting options:
* {@code sep}: the key-value pair separator
* {@code quote}: the character to be used to quote the parameter value
* {@code prefix}: the character(s) prepended to the parameter key
*
* @return A command line formatted string e.g. {@code --foo x --bar y}
*/
String all(Map opts = null) {

String sep = opts?.sep ?: ' '
String quote = opts?.quote ?: ''
String prefix = opts?.prefix ?: '--'

def result = []
realNames.each {
result << ("$prefix$it$sep${wrap(this.get(it).toString(), quote)}".toString())
}
return result.join(' ')
}

/**
* Wrap a string value with quote characters
*
* @param str The string with value to be wrapped by quote characters
* @param quote The quote character
* @return The quoted string
*/
@PackageScope
static String wrap( String str, String quote ) {
if( !quote && (str.contains(' ') || str.contains('"') || str.contains("'") ))
quote = '"'

if( !quote )
return str

def result = new StringBuilder()
result.append(quote)
str.each {
result.append( it == quote ? '\\'+quote : it )
}
result.append(quote)
return result.toString()
}

/**
* Converts a string containing words separated by a hyphen character to a camelCase string
*
* @param str The string to be converted
* @return
*/
@PackageScope
static String hyphenToCamelCase( String str ) {

if( !str ) { return str }

def result = new StringBuilder()
str.split('-').eachWithIndex{ String entry, int i ->
result << (i>0 ? StringUtils.capitalize(entry) : entry )
}

return result.toString()
}

/**
* Converts a camel-case string to a string where words are separated by hyphen character
*
* @param str The string to be converted
* @return A string where camel-case words are converted to words separated by hyphen character
*/
@PackageScope
static String camelCaseToHyphen( String str ) {

def lower = 'a'..'z'
def upper = 'A'..'Z'

def result = new StringBuilder()
if( !str ) {
return str
}

result << str[0]
for( int i=1; i<str.size(); i++ ) {
if( str[i] in upper && str[i-1] in lower ) {
result << '-'
if( i+1<str.size() && str[i+1] in lower ) {
result << str[i].toLowerCase()
}
else {
result << str[i]
}
}
else {
result << str[i]
}
}

return result.toString()
}

}

}
30 changes: 30 additions & 0 deletions modules/nextflow/src/test/groovy/nextflow/cli/CmdRunTest.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,36 @@ class CmdRunTest extends Specification {
[:] | /x.y\.z/ | 'Hola' | ['x': ['y.z': 'Hola']]
}

def 'should convert cli params from kebab case to camel case' () {

when:
def params = [:]
CmdRun.addParam0(params, 'alphaBeta', 1)
CmdRun.addParam0(params, 'alpha-beta', 10)
then:
params['alphaBeta'] == 10
!params.containsKey('alpha-beta')

when:
params = [:]
CmdRun.addParam0(params, 'aaa-bbb-ccc', 1)
CmdRun.addParam0(params, 'aaaBbbCcc', 10)
then:
params['aaaBbbCcc'] == 10
!params.containsKey('aaa-bbb-ccc')

}

def 'should convert kebab case to camel case' () {

expect:
CmdRun.kebabToCamelCase('a') == 'a'
CmdRun.kebabToCamelCase('A') == 'A'
CmdRun.kebabToCamelCase('a-b-c-') == 'aBC'
CmdRun.kebabToCamelCase('aa-bb-cc') == 'aaBbCc'
CmdRun.kebabToCamelCase('alpha-beta-delta') == 'alphaBetaDelta'
CmdRun.kebabToCamelCase('Alpha-Beta-delta') == 'AlphaBetaDelta'
}

@Unroll
def 'should check run name #STR' () {
Expand Down
Loading

0 comments on commit 349f4f6

Please sign in to comment.