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.9k stars 238 forks source link

The `newline` option in the template module does not behave according to documentation #466

Closed RiskoZoSlovenska closed 7 months ago

RiskoZoSlovenska commented 7 months ago

The documentation for template.compile states that

newline: string to replace newline characters, default is nil (not replacing newlines).

However, as can be seen in the code, a truthy value simply replaces all newlines with an empty string, and a falsy value does not:

    if newline then
        r = format("%q", strgsub(strsub(chunk,s),"\n",""))
    else

In other words, passing in a string different from the empty string does not actually replace the newlines with that string, contrary to what the documentation says.

I'd gladly PR a fix, but I'm not sure what the intended behaviour was actually meant to be; is it the documentation or the code that needs to be changed?

alerque commented 7 months ago

Fixing the docs should be pretty straight forward. Changing the behavior might be a bit trickier because depending on the change it might be considered breaking. What is the really useful use case here? And is there a way to make it do what you want without possible breaking existing code?

RiskoZoSlovenska commented 7 months ago

I don't need different behaviour myself (I can't think of a use-case where you'd want to replace newlines with something else), I just noticed the discrepancy while trying to implement an unrelated enhancement. Simply updating the docs would be fine. Renaming the option to something like strip_newlines (and supporting the old name too for compat) would also be nice, but not necessary.

alerque commented 7 months ago

If you don't have a use case for something else, lets just fix the docs to document what we do. That will be a docs fix not a potentially breaking change.

Care to submit a PR?

RiskoZoSlovenska commented 7 months ago

Sure