Make indent preserve the of \n in the input string (stop adding a trailing \n)

[jRimbault]

Wait for split_inclusive to be stable before merging.

closes #207

[mgeisler]

I tried to use the add_nl helper function together with a vector to align the lines in the source code and to make it crystal clear if a string has trailing whitespace. I would prefer to keep this somehow, but I'm open for better ways of doing it :-)

[jRimbault]

What do you think would be good way to signal trailing whitespaces are intentional ?

[mgeisler]

Hmm... yeah, good question. I guess the most direct way would be to include things like \n in the strings, and then join them without adding a separator. So perhaps we can throw out the add_nl function and let the tests be explicit:

        let lines = &[
            "foo\n", //
            "bar\n",
        ];
        let expected = &[
            "  foo\n", //
            "  bar\n",
        ];
        assert_eq!(&indent(&lines.join(""), "  "), &expected.join("")); 

the // makes rustfmt align things consistently so that it is easy to compare the two lists visually. I like that each line is shown in full, with escape characters and everything. Trailing whitespace would then be

        let lines = &[
            "foo  \n", //
            "bar    ",
        ];
        let expected = &[
            "  foo  \n", //
            "  bar    ",
        ];
        assert_eq!(&indent(&lines.join(""), "  "), &expected.join("")); 

If you want, you could try updating the existing tests to use this scheme. It would be a good preparation for this PR.

[jRimbault]

I'll get on it Monday :)

And thanks for the review !

Edit: had time tonight, so...

[mgeisler]

Thanks, the explicit lists look good!

Can I get you to squash all your commits into a single commit with a commit message such as

Make indent preserve the lack of a trailing \n

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

You could also mention that we now require Rust version N.NN (when we know this at some point).

[jRimbault]

Sure :)

[mgeisler]

I just noticed that a stabilization PR has been put up for the split_inclusive feature: rust-lang/rust#77858 — I'm not sure how long it will take to be reviewed, though.

[mgeisler]

This was solved in a simpler way in #279 so I'm closing this now. Thanks @jRimbault!