Allow write_file to sanitize spaces and newlines #165
Conversation
ssbarnea
force-pushed
the
feat/sanitize
branch
from
0c8d7a1 to
17cd5a0
Compare
ssbarnea
force-pushed
the
feat/sanitize
branch
from
17cd5a0 to
53b17d7
Compare
There was a problem hiding this comment.
I think this is the wrong place for such functionality. This should be done by the caller before calling write_file().
Also the implementation is very inefficient. Check for example with " "*100000 + "x" + " "*100000. Compare for example to content = '\n'.join([l.rstrip(' \t') for l in content.splitlines()]), which is so much faster for this example.
| @@ -71,10 +72,17 @@ async def copy_file( | |||
| flog.debug("Leave") | |||
|
|
|||
|
|
|||
| async def write_file(filename: StrOrBytesPath, content: str) -> None: | |||
| async def write_file( | |||
| filename: StrOrBytesPath, content: str, /, sanitize: bool = False | |||
There was a problem hiding this comment.
Technically speaking, making filename and content positional-only arguments is a breaking change.
| if sanitize: | ||
| content = re.sub(r"[ \t]+$", "", content, flags=re.MULTILINE) | ||
| content = content.rstrip("\n\r") + "\n" |
There was a problem hiding this comment.
I agree with Felix that perhaps this isn't the best place to do this, but maybe a separate sanitize_trailing_spaces() function in antsibull_core would make sense
| @@ -0,0 +1,2 @@ | |||
| minor_changes: | |||
| - Allow write_file to sanitize spaces and newlines. (https://github.com/ansible-community/antsibull-docs/issues/305) | |||
There was a problem hiding this comment.
| - Allow write_file to sanitize spaces and newlines. (https://github.com/ansible-community/antsibull-docs/issues/305) | |
| - Allow write_file to sanitize spaces and newlines (https://github.com/ansible-community/antsibull-docs/issues/305). |
is the format we generally follow.
|
Is this still relevant? I think @ssbarnea's original issue was solved differently in antsibull-docs? |
|
Either way, thank you for the contribution. |
|
Closing since this is apparently no longer needed. Thanks for your contribution! |
Needed-By: ansible-community/antsibull-docs#305