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

Dedent() remove surrounding newlines #7

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

kindrowboat
Copy link

Breaking change! I realise that this would be a breaking change that you might not desire. I'm happy using my fork of this library for my own projects, but I wanted to know your thoughts on this, and if you'd be interested in merging it in or altering this to be a backward compatible change (e.g. adding a new method that does the Dedent + newline trimming.)


The most common use-case for Dedent() is a situation like:

multiLineString = dedent.Dedent(`
    This is a
    multi-line string.
`)

Most programmers would reasonably expect multiLineString to be equal to "This is a\nmulti-line string." Currently Dedent() returns "\nThis is a\nmulti-line string.\n". When using raw string syntax, there is currently no good way to specify a multi-line string without a "\n". This commit strips a "\n" prefix and suffix, only if the string has both a "\n" prefix and a "\n" suffix.

The most common use-case for Dedent() is a situation like:

    multiLineString = dedent.Dedent(`
        This is a
        multi-line string.
    `)

Most programmers would reasonably expect multiLineString to be equal to
`"This is a\nmulti-line string." Currently Dedent() returns `"\nThis is
a\nmulti-line string.\n". When using raw string syntax, there is currently no good way to specify a multi-line string without a "\n". This commit strips a "\n" prefix and suffix, only if the string has both a "\n" prefix and a "\n" suffix.
@lithammer
Copy link
Owner

lithammer commented Jul 31, 2020

Thanks for taking your time with this. I think I need to think about this for a bit, mainly because this was initially created to mimic Python's textwrap.dedent() function. And it includes the newline in both ends:

>>> import textwrap
>>> textwrap.dedent("""
...     This is a
...     multi-line string.
... """)
'\nThis is a\nmulti-line string.\n'

On the other hand, stripping the newlines is just a str.strip() call away in Python. So given that, one could argue it's just as easy to call strings.TrimSpace() in Go. While going the other way is tricker/uglier. Hmm... 🤔

@kindrowboat
Copy link
Author

I recently found another package, heredoc, that seems to solve my use-case. If you want to keep this package a pure port of textwrap.dedent(), I don't blame you. Feel free to close this PR.

Thank you again for this package. It's made my tests (especially in Ginkgo) more readable.

@lithammer
Copy link
Owner

lithammer commented Aug 5, 2020

Interesting, seems almost identical except that it trims whitespace from the beginning of the string. I will keep it open for while longer while I ponder things 🧐

I mean, I kind of get the appeal, so adding another method might not be the worst thing in the world. Something like Dedentf for formatting looks pretty cool as well. Which I noticed the heredoc package had.

Thank you again for this package. It's made my tests (especially in Ginkgo) more readable.

I'm glad to hear that 😊

@kindrowboat
Copy link
Author

Yeah, I was thinking of what the name of such a second method could be. The obvious choice would be dedent.Heredoc(), but in a way that's misleading because HEREDOCs in most languages (BASH, PHP, and Ruby for sure) preserve any indentation similar to Golang's raw string. Ruby introduced the "squiggly heredoc" in Ruby 2.3 that essentially does what heredoc.Heredoc is doing, but I'm not sure if you want to have a method called dedent.SquigglyHeredoc(). 😉

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

Successfully merging this pull request may close these issues.

2 participants