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

Option to indent the shell section #39

Closed
enixmail opened this issue Apr 29, 2020 · 9 comments
Closed

Option to indent the shell section #39

enixmail opened this issue Apr 29, 2020 · 9 comments
Assignees

Comments

@enixmail
Copy link

This is just a suggestion, for when you'll be at the point to insert "nice to have" items.

I saw your discussion about not indenting the inside of multi-lines string shell sections
#35 (comment)
and while I understand your point of view, I was also very disappointed as I was counting on that feature.

For an obscure reason, when I started building Snakemake workflows, I used a 2 spaces indentation instead of 4, so I have several Snakefiles following this pattern:

rule rule1:
  output:
    "output.txt"
  shell:
    """
    echo Hello
    touch {output[0]}
    """

which I was hoping snakefmt would fix (the indentation) this way:

rule rule1:
    output:
        "output.txt",
    shell:
        """
        echo Hello
        touch {output[0]}
        """

It could be nice to have a command line argument like --force-shell-indent to have snakefmt also adjust the indentation of the lines inside the multi-lines string shell sections.

Thanks!

@johanneskoester
Copy link
Contributor

I agree, it definitely should be indented this way. And this should be the default. Otherwise, it seriously hampers readability. I think this should be doable using textwrap from the standard library.

@johanneskoester
Copy link
Contributor

cc @mbhall88 @bricoletc

@johanneskoester
Copy link
Contributor

johanneskoester commented Apr 30, 2020

I think this should work:

textwrap.indent(textwrap.dedent(shellcmd.strip()), " " * shell_indentation_level)

@bricoletc bricoletc self-assigned this Apr 30, 2020
bricoletc added a commit that referenced this issue May 2, 2020
* Triple quoted strings now get reindented with the project-wide used indentation
* Relative indents between lines of the string are preserved
@bricoletc
Copy link
Collaborator

Hey @enixmail , I think this is fixed with 0499a7f, you get your two spaces reindented properly!

Note I reindent by the lowest indented element, and then relative indenting is maintained so on:

rule rule1:
  shell:
    """
 echo Hello
    touch {output[0]}
    echo World
"""

(Hard to see, but a single space before echo Hello)
The output is

rule rule1:
    shell:
        """
        echo Hello
           touch {output[0]}
           echo World
        """

@enixmail
Copy link
Author

enixmail commented May 3, 2020

With commit 0499a7f, the shell sections are now well indented, but I think that docstrings might have been affected.

Using this Snakefile:

def foo():
  """This is a Python function docstring summary line

  Here is the description portion of
  the docstring.
  """
  print("Hello")


rule all:
  """This is a rule docstring summary line

  Here is the description portion of
  the docstring.
  """
  output:
    "output.txt",
  shell:
    """
    echo Hello
    touch {output[0]}
    """

I expect to get that as output from snakefmt:

def foo():
    """This is a Python function docstring summary line

    Here is the description portion of
    the docstring.
    """
    print("Hello")


rule all:
    """This is a rule docstring summary line

    Here is the description portion of
    the docstring.
    """
    output:
        "output.txt",
    shell:
        """
        echo Hello
        touch {output[0]}
        """

but currently the output is this:

def foo():
    """This is a Python function docstring summary line

  Here is the description portion of
  the docstring.
"""
    print("Hello")


rule all:
    """This is a rule docstring summary line

      Here is the description portion of
      the docstring.
    """
    output:
        "output.txt",
    shell:
        """
        echo Hello
        touch {output[0]}
        """

@bricoletc
Copy link
Collaborator

bricoletc commented May 3, 2020

Hey @enixmail , thanks for the follow-up. I've made a fix relating to python code but don't think we should be reindenting your docstring.

If you run black on :

def foo():
  """This is a Python function docstring summary line

  Here is the description portion of
  the docstring.
  """
  print("Hello")

You get:

def foo():
    """This is a Python function docstring summary line

  Here is the description portion of
  the docstring.
  """
    print("Hello")

Which is what i've implemented with a coming commit.
Under shell I agree we should reformat triple quoted strings for readability, but i think we should generally maintain that triple quoted strings get left as the coder wrote them.

Open to discussion of course!

@bricoletc
Copy link
Collaborator

Another option is to reformat all triple quoted strings and not do it as black there.

bricoletc added a commit that referenced this issue May 3, 2020
With unit test
As per #39 continued
@enixmail
Copy link
Author

enixmail commented May 4, 2020

I misread PEP 257 and was under the (false) impression that the text of all the other lines within the multi-lines docstring had to be left aligned with the start of the opening triple double quotes, which is not the case. Looking at Black's issues list, docstrings seem to have fueled many discussions, I guess snakefmt should simply stick to what black is doing with docstrings.
Thanks!

bricoletc added a commit that referenced this issue May 5, 2020
* Triple quoted string formatting only on parameter values (#39)
* Top-level elements trigger two newlines for spacing (#38)
	Elements are python code and snakemake keywords
  With simplified, and added, unit tests
@bricoletc
Copy link
Collaborator

Sure!

bricoletc added a commit that referenced this issue May 10, 2020
* Triple quoted strings now get reindented with the project-wide used indentation
* Relative indents between lines of the string are preserved
bricoletc added a commit that referenced this issue May 10, 2020
With unit test
As per #39 continued
bricoletc added a commit that referenced this issue May 10, 2020
* Triple quoted string formatting only on parameter values (#39)
* Top-level elements trigger two newlines for spacing (#38)
	Elements are python code and snakemake keywords
  With simplified, and added, unit tests
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