prefix() proposal. Closes broadinstitute/cromwell#375 #84

Merged
merged 1 commit into from Jan 17, 2017

Conversation

Projects
None yet
3 participants
Contributor

mcovarr commented Jan 9, 2017 edited

This is an attempt to solve the problem presented here of composing a string from a prefix string and a list of zero or more value strings. There are definitely a lot of ways this problem might be solved and this is a concrete proposal just to get a discussion started. 😄

Pinging @slnovak @vdauwera @knoblett @katevoss @jsotobroad @LeeTL1220 @yfarjoun

mcovarr referenced this pull request in broadinstitute/cromwell Jan 10, 2017

Closed

Support additional Docker configuration options. #375

Contributor

Horneth commented Jan 10, 2017

I also like the idea of having an mkString for arrays in general. I find the scala one pretty simple and complete but it doesn't have to be the same.
http://www.scala-lang.org/api/2.10.3/#scala.collection.GenTraversableOnce

And we could still have prefix, suffix sugar method on top.

@cjllanwarne

Like @Horneth I can see the value of a bumper all-in-one mkString(prefix: String, separator: String, suffix: String, array: Array[String]) which might make this redundant. But this still looks good to me 👍

SPEC.md
@@ -94,6 +94,7 @@
* [Array\[Array\[X\]\] transpose(Array\[Array\[X\]\])](#arrayarrayx-transposearrayarrayx)
* [Pair(X,Y) zip(X,Y)](#pairxy-zipxy)
* [Pair(X,Y) cross(X,Y)](#pairxy-crossxy)
+ * [String prefix(String, Array\[String\])](#string-prefix-string-arraystring)
@cjllanwarne

cjllanwarne Jan 12, 2017

Contributor

This link didn't work for me

@mcovarr

mcovarr Jan 12, 2017

Contributor

Whoops, that's because I have a superflous dash. Thank you for reviewing so thoroughly! 😄

SPEC.md
+
+```
+Array[String] env = ["key1=value1", "key2=value2", "key3=value3"]
+String env_param = prefix(" -e ", env) # " -e key1=value1 -e key2=value2 -e key3=value3"
@cjllanwarne

cjllanwarne Jan 12, 2017

Contributor

Nice! The extra space at the start is awkward but (shrug)

Contributor

mcovarr commented Jan 12, 2017

@cjllanwarne a Scala-esque mkString wouldn't make this redundant since mkString only inserts strings between elements of the array, it doesn't prepend the strings to every element of the array. For this particular use case, I want a completely blank string if the array is empty, and -e key1=value if there's one element in the array.

Having said that I'm happy to make mkString, prefix, and suffix functions as @Horneth suggested if we're agreed those would be a good idea.

Contributor

cjllanwarne commented Jan 12, 2017

@mcovarr good point! I think prefix adds by far the most value. mkString is nice (especially if it means we can retire the ${sep=" " array} syntax although I would prefer a better name. Suffix, meh, but there might be uses for it so why not...

Contributor

Horneth commented Jan 14, 2017 edited by briandmaher

👍

Approved with PullApprove

Contributor

cjllanwarne commented Jan 17, 2017 edited by briandmaher

👍

Approved with PullApprove

@mcovarr mcovarr merged commit 944ebea into develop Jan 17, 2017

1 check passed

code-review/pullapprove Approved by cjllanwarne, Horneth
Details

mcovarr deleted the mlc_join branch Jan 17, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment