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

Implement stdin-stdout operation with '-' file argument #119

Merged
merged 2 commits into from
Nov 4, 2024

Conversation

AndydeCleyre
Copy link
Contributor

@AndydeCleyre AndydeCleyre commented Oct 26, 2024

This works well in my casual testing, and can even be used in combination with regular file arguments. But I haven't written a formal test for it yet, so I'm marking this as a draft.

It makes sure to write the stdin to stdout if there's a problem formatting/encoding/decoding, and if no formatting is necessary. This ensures it doesn't delete content when used as one step in a pipeline of successive formatters and linters.

@AndydeCleyre
Copy link
Contributor Author

I have pylint checks failing locally, but for files I haven't changed.

@AndydeCleyre AndydeCleyre marked this pull request as ready for review October 27, 2024 03:28
@AndydeCleyre AndydeCleyre marked this pull request as draft October 27, 2024 05:07
@AndydeCleyre

This comment was marked as resolved.

@AndydeCleyre AndydeCleyre marked this pull request as ready for review October 27, 2024 05:58
@@ -56,20 +58,29 @@ def main():
for path in find_python_files(args.files):
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be worth adding something to find_python_files to prevent it from searching directories named '-'.

I think I agree that it makes sense to allow mixing '-' with other arguments.

@bwhmather
Copy link
Owner

Looks good. I recon fix find_python_files to skip '-' then I'll merge and bake a new release.

if the content is already ssort-ed, the output is empty. When used as a step in a multiple-formatter pipeline, this leads to deleting the content.

Lol.

@coveralls
Copy link

coveralls commented Oct 28, 2024

Pull Request Test Coverage Report for Build 11673472686

Details

  • 25 of 30 (83.33%) changed or added relevant lines in 2 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage decreased (-0.1%) to 98.132%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/ssort/_main.py 24 29 82.76%
Files with Coverage Reduction New Missed Lines %
src/ssort/_main.py 1 92.0%
Totals Coverage Status
Change from base Build 11673464849: -0.1%
Covered Lines: 1366
Relevant Lines: 1392

💛 - Coveralls

@bwhmather
Copy link
Owner

I have pylint checks failing locally, but for files I haven't changed.

Yup. Not your fault. pylint not pinned. This is something we've chatted about fixing. Not the fastest moving project so stuff like this sadly inevitable. Have raised #120.

@bwhmather bwhmather force-pushed the feature/stdin-stdout branch from b8eda54 to 39593a8 Compare November 4, 2024 22:05
@bwhmather bwhmather merged commit 6e66c2c into bwhmather:master Nov 4, 2024
22 checks passed
@bwhmather
Copy link
Owner

Thank you very much. I've pushed v0.14.0 to pypi with this change.

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.

3 participants