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

Option to error on cell outputs exceeding --max-size #167

Open
janosh opened this issue Aug 19, 2022 · 4 comments
Open

Option to error on cell outputs exceeding --max-size #167

janosh opened this issue Aug 19, 2022 · 4 comments
Labels
help wanted state:waiting Waiting for response for reporter type:enhancement
Milestone

Comments

@janosh
Copy link
Contributor

janosh commented Aug 19, 2022

The --max-size flag added in #135 is a great addition! 👍

When using nbstripout as commit hook, it would be nice to have a slightly safer mode that doesn't auto-remove output exceeding the max size but rather raises an error to abort the commit and let the developer handle it on a case by case basis (maybe compress rather than remove the cell output).

@kynan
Copy link
Owner

kynan commented Sep 24, 2022

I can definitely see that use case, however isn't the point (and mode of operation) of the pre-commit hook that it'll fail if it would make any changes to the working tree? Correct me if I'm wrong, as I don't use nbstripout in this mode, so I'm stating this based on some light testing I'd done quite a while ago and some other issues that have been raised on the pre-commit use case.

@kynan kynan added the state:waiting Waiting for response for reporter label Sep 24, 2022
@janosh
Copy link
Contributor Author

janosh commented Oct 1, 2022

pre-commit hooks can be both linters and formatters. Formatters should fix violations themselves, linters only flag them. Any hook can also be a combination of both, i.e. fix some violations automatically and report others.

@kynan
Copy link
Owner

kynan commented Oct 2, 2022

Fair enough! Are you interested in working on this @janosh ? As I don't use nbstripout in this mode I think this would ideally be implemented by someone who can test it and ensure the feature works as intended.

@kynan
Copy link
Owner

kynan commented Feb 5, 2023

I won't be able to work on implementing this. Contributions welcome!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted state:waiting Waiting for response for reporter type:enhancement
Projects
None yet
Development

No branches or pull requests

2 participants