-
Notifications
You must be signed in to change notification settings - Fork 306
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
Multi-line strings #414
Multi-line strings #414
Conversation
@jdidion This is amazing. I believe this will be considered a highly welcome addition to the WDL specification! |
|
We could also have a |
My feeling was that the most common usage for multi-line strings will be to break up long strings. If you want to write bash, you put that in the command block, no? I don't think we need a special |
Coming from scala might be distorting my sense of "what people expect" w.r.t. newlines in multi-line strings. Usually the "what would people expect" tie-breaker in cases of different aesthetic preferences is "what would python do", which seems appropriate here too. |
As far as I can tell, python also preserves newlines in multi-line string literals (https://www.techbeamers.com/python-multiline-string/). Unless there's a really strong case against it, I think that should be what we do. Yes, I think bash commands usually go in command blocks, but I still think having a syntax that erases very common bash-style syntax by default in multi-line string literals feels dangerous to me |
Personally, I find it very annoying to have to litter my Scala code with I agree that "least surprising" is a good approach, but I also feel it can be over-ridden by the desire to simplify the most common use case. We can add more examples to make it clear that if you want to write a multi-line bash string (or whatever) that preserves newlines you have to use That said, I admit I may be wrong about what will be the most common use-case. |
I'd argue with your premise there though that the most common use case is "I want to split a single line string across many lines". I think the most common usage would be "I want a to write a multi-line string directly in my WDL". In which case the simplest way to provide that is to make it the default. But we're arguing our own personal preferences here - which will never have a definitive answer. I think "choose the options that's most similar to python" is the most objective possible way to resolve these discussions. |
Fair enough. Let's wait to get a bit more feedback from others. I'm happy to update the spec and grammar if the consensus is to leave in whitespace by default. |
Agreed! An n > 2 is always a good idea... |
Personally, I find the least surprising thing to be represent the string "as-is" with new lines intact. for me that is the main point of a multiline string, is to write across multiple lines. The tricky part I see is around preserving indents or removing them. I believe in python (since its tab/space delimeted) this is easy, since you simply strip out leading tabs corresponding to the current indent level. In WDL this is a bit trickier. we could
|
Ok, how about:
|
I really worry about strings like
|
Oh, good point. In that case I guess we may need a
|
A simpler alternative would be a |
Those ideas all sound reasonable to me. |
Have you looked at the spec for Java text blocks? The java spec supports both automatically stripping newlines and preserving relative indent and doesn't require a lot of extra syntax. https://openjdk.java.net/jeps/378 |
I do like that java text block spec's interpretation of "intrinsic whitespace", which matches very closely the |
Ive just started using Java's implementaiton of text blocks and they are very intuitive, so I give a big +1 to that! |
is the discussion on how to handle whitespace still live or do we have general consensus? |
This would be really helpful, we have a large CWL pipeline we want to convert to WDL which uses YAML text blocks extensively for SQL queries. Not having an equivalent in WDL has been frustrating, as the SQL needs to be squashed into a single line. This feature would be greatly appreciated. |
@jdidion any update on the status of this? It would be great to get this to voting |
FWIW I'd vote for the Java text block syntax. Since my last post we've resorted to a yaml → wdl converter which isn't fun. |
👍 to the usefulness of this feature! |
I am good with Java text block syntax. I haven't read the spec in detail but it looks to me like the important things are:
The only thing that is not clear to me is how to handle mixed indentation. For example, is
Preference? |
Included this feature in miniwdl v1.8.0 |
@mlin great! I'm personally satisfied with the spec for this feature. I'll tag people for reviews. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks excellent. Great work @jdidion !
LGTM |
This looks great! Amazing effort from everyone on getting to a design that feels perfect! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor formatting issue, but otherwise 👍
This is a | ||
multi-line string! | ||
>>> | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing closing ``` backticks here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed, thanks!
Last call for comments. Plan to merge later this week. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚢
🫡 |
The ability to have multi-line strings has been requested several times. It would also make easier several of the other proposals under consideration, such as in-line Dockerfile and evaluation of arbitrary expressions. I went with Python/Scala-style rather than HEREDOC style as that seems most idiomatic for WDL.
This is implemented in the v2 ANTLR4 grammar in a wdlTools branch: https://github.com/dnanexus-rnd/wdlTools/tree/multiline-strings/src/main/antlr4/v2
Checklist