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

The indent function always adds a final \n, even if the input string has no \n #207

Closed
jRimbault opened this issue Sep 17, 2020 · 3 comments · Fixed by #279
Closed

The indent function always adds a final \n, even if the input string has no \n #207

jRimbault opened this issue Sep 17, 2020 · 3 comments · Fixed by #279
Labels

Comments

@jRimbault
Copy link
Contributor

jRimbault commented Sep 17, 2020

Example:

// this test fails currently
#[test]
fn indented_text_should_have_the_same_number_of_lines_as_the_original_text() {
    let texts = ["foo\nbar", "foo\nbar\n", "foo\nbar\nbaz"];
    for original in texts.iter() {
        let indented = indent(original, "");
        assert_eq!(&indented, original);
    }
}

There should be the same number of lines in the input and output (unless the prefix contains a newline).

Python tests reference, python source reference, can't be implemented the same way in rust without the split_inclusive feature. (cf. python's splitlines)

Python script passes the same test
from textwrap import indent

for original in ["foo\nbar", "foo\nbar\n", "foo\nbar\nbaz"]:
    indented = indent(original, "")
    assert original == indented
jRimbault added a commit to jRimbault/textwrap that referenced this issue Sep 17, 2020
jRimbault added a commit to jRimbault/textwrap that referenced this issue Sep 17, 2020
jRimbault added a commit to jRimbault/textwrap that referenced this issue Sep 17, 2020
also adds part of the python indent test suite
@mgeisler
Copy link
Owner

Hi @jRimbault Thank you very much for the excellent bug report and the accompanying fixes!

Since it uses a nightly API, I guess we'll just let is sit and wait for a bit...?

In the mean time, I'll have to look at why the tests didn't actually break as expected! I recently switched to GitHub actions, but I must have misconfigured it somehow :-)

If I read #208 and #209 correctly, then I think #208 is a subset of #209? If so, I would prefer to close #208 and simply merge the tests together with the implementation.

@mgeisler mgeisler changed the title indent is adding a last newline The indent function always adds a final \n, even if the input string has no \n Sep 18, 2020
@jRimbault
Copy link
Contributor Author

jRimbault commented Sep 18, 2020

I made #208 in case you wanted to merge the tests in some working branch of yours ? It's really only just in case it's useful to you. If you look at #209's history detail you'll see I made other partial fixes (559df5c) before using the nightly feature, but I couldn't figure it out completely. I thought you should have a go at fixing it yourself if interested ^^ of course you can close it

I think waiting for the split_inclusive to be in stable shouldn't take too long, it's a simple feature and it's only missing a stabilization PR.

PS: in your workflow you only need

on: [push, pull_request]

mgeisler added a commit that referenced this issue Sep 26, 2020
Thanks to @jRimbault for explaining how to fix this in #207.
mgeisler added a commit that referenced this issue Sep 26, 2020
Thanks to @jRimbault for explaining how to fix this in #207.

To avoid running no less than eight checks whenever I push something
to a new branch, we now only run the build workflow on pushes to the
master branch.
@mgeisler
Copy link
Owner

Thanks for the tip about fixing the GitHub workflow!

I don't have a working branch as such, so I'll just close #208 for now.

jRimbault added a commit to jRimbault/textwrap that referenced this issue Sep 30, 2020
Before, indent("foo", "") would give "foo\n".
It now preserves any trailing newline character present in the
input string. This makes indent behave consistently with dedent.
New tests ere added to ensure this on a number of corner cases.

closes mgeisler#207
@mgeisler mgeisler added the bug label Oct 26, 2020
mgeisler pushed a commit that referenced this issue Jan 21, 2021
…iling \n) [stable rustc] (#279)

* Make indent preserve newlines or lack thereof

Before, indent("foo", "") would give "foo\n".
It now preserves any trailing newline character present in the
input string. This makes indent behave consistently with dedent.
New tests ere added to ensure this on a number of corner cases.

Closes #207.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants