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

Added linebreak option for indent filter. #2031

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

mjeromin
Copy link

@mjeromin mjeromin commented Oct 2, 2024

This change fixes the indent filter so that it can support any line break sequence, including line feed (LF), carriage return line feed (CRLF), and carriage return (CR), whichever newline sequence is set in the environment.

#2016

fixes #2016

👋 This is my first time contributing, please advise if an entry to CHANGES.rst should be added and which version it should be associated (e.g. 3.2.0).

@davidism
Copy link
Member

davidism commented Oct 2, 2024

The issue points out that the environment already has a setting for the line ending. That should be used instead of adding a filter argument.

@jmbraben
Copy link

jmbraben commented Oct 29, 2024

As stated in #2016 , the changes addressed my use case. Was any search for any other instances of hardcoded use of '\n' made?

@davidism
Copy link
Member

davidism commented Oct 29, 2024

Was any search for any other instances of hardcoded use of '\n' made?

I don't think so. If you (or someone else) want to look, and find other places that should use the env setting, that would be another good issue to open.

@jmbraben
Copy link

There is nothing else in filters.py, the only thing in /src I'm questioning is:

self.stream.write("\n" * self._new_lines)

But as in the compiler.py, thinking it is not an issue.

@davidism
Copy link
Member

You're right, that's not an issue.

@jmbraben
Copy link

jmbraben commented Nov 15, 2024

@mjeromin Is there any timeline to merge this?

@mjeromin
Copy link
Author

mjeromin commented Nov 15, 2024

@jmbraben We confirmed the fix in the issue #2016, we can go ahead and merge. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

indent filter uses \n always rather than following api newline_sequence
3 participants