Skip to content
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

Removing whitespace in WDL doesn't work for overindented code blocks #4983

Closed
stxue1 opened this issue Jun 22, 2024 · 4 comments
Closed

Removing whitespace in WDL doesn't work for overindented code blocks #4983

stxue1 opened this issue Jun 22, 2024 · 4 comments
Assignees

Comments

@stxue1
Copy link
Contributor

stxue1 commented Jun 22, 2024

The spec seems to want runners to support stripping whitespace from overindented python code blocks:
https://github.com/openwdl/wdl/blob/9c0b9cf4586508a9e6260cc5c5e562e21f625aac/SPEC.md?plain=1#L3590-L3624

The current issue is the amount of whitespace to remove is done at the first seen line:

common_whitespace_prefix = line_whitespace_prefix

In the spec example's case, it finds 2 spaces at python <<CODE but needs to remove 4 at with open("~{infile}") as fp:

┆Issue is synchronized with this Jira Story
┆Issue Number: TOIL-1596

@adamnovak
Copy link
Member

Does Cromwell handle this somehow?

I would think the spec is wrong here; I don't think that that shell command with just its internal indentation can work. You might want to indent inside the heredoc, but if Bash doesn't make allowances for it, and Python doesn't let you indent the entire script, I don't know why it makes sense for the WDL interpreter to provide special logic to give you this feature.

@adamnovak
Copy link
Member

The spec doesn't actually say that the given example works, just that it produces a particular Bash script:

python <<CODE
  with open("/path/to/file") as fp:
    for line in fp:
      if not line.startswith('#'):
        print(line.strip())
CODE

That Bash script runs Python with an input where the first line starts with two spaces. Python then raises an error.

@adamnovak
Copy link
Member

I've reported a bug on the spec: openwdl/wdl#668

@adamnovak
Copy link
Member

The WDL spec is going to change with openwdl/wdl#669 to no longer suggest over-indenting heredocs in its examples, so I am going to close this.

@adamnovak adamnovak closed this as not planned Won't fix, can't repro, duplicate, stale Jul 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants