lunarmodules / Penlight

A set of pure Lua libraries focusing on input data handling (such as reading configuration files), functional programming (such as map, reduce, placeholder expressions,etc), and OS path management. Much of the functionality is inspired by the Python standard libraries.
https://lunarmodules.github.io/Penlight/
MIT License
1.93k stars 241 forks source link

stringx indent and dedent functions add an extra "\n" at the end of the string #446

Open Aire-One opened 2 years ago

Aire-One commented 2 years ago

Hello,

The indent and dedent functions both add a final "\n" sequence at the end of the string.

Code reference of the indent function: https://github.com/lunarmodules/Penlight/blob/b101290c532c2901764de57367d3f2b84127286d/lua/pl/stringx.lua#L500-L514

I think this extra "\n" sequence shouldn't be here. It's not part of the functions documentations and is (at least in my use case) unwanted.

Tieske commented 1 year ago

I tend to agree that the extra newline is weird. But is has been in here for 13 years. So not easy to fix, since it is breaking.

Maybe we should label this as something to fix in a next major version.

Tieske commented 1 year ago

This equally applies to dedent, and looking at the code it makes me wonder whether this even has a clean solution. What is the expected output for in input that has NO trailing newline? That's easy, last line gets indented, and no trailing newline. Now what if the input does have a trailing newline? should that newline now be indented as well?

Aire-One commented 1 year ago

I tend to agree that the extra newline is weird. But is has been in here for 13 years. So not easy to fix, since it is breaking.

This equally applies to dedent, and looking at the code it makes me wonder whether this even has a clean solution.

With a second look at the code, I can see that there are even more of the stringx functions returning an extra \n.

What is the expected output for in input that has NO trailing newline? That's easy, last line gets indented, and no trailing newline.

Agree. I think it should be indent("hello\nworld", 2) == " hello\n world".

Now what if the input does have a trailing newline? should that newline now be indented as well?

Hm, I think it should, yes. It would be indent("hello\nworld\n", 2) == " hello\n world\n ".

I would let the caller have the responsibility to trim the end of the string before calling indent/dedent (if they need to remove the extra line).