nbstripout 0.8.2 can fight with pre-commit-hooks v6.0.0 on Windows over line endings

[owenlamont]

I have the follow prek/pre-commit config:

repos:
  - repo: https://github.com/kynan/nbstripout
    rev: 0.8.2
    hooks:
      - id: nbstripout
        args: [--drop-empty-cells]
  - repo: https://github.com/pre-commit/pre-commit-hooks
    rev: v6.0.0
    hooks:
      - id: mixed-line-ending
        args: [--fix=lf]

Which works fine on Ubuntu but when I tried it on Windows nbstripout would start adding carriage returns to my Jupyter notebooks and pre-commit-hooks would remove them again in an endless cycle.

Could you add some config option to specify a preferred line ending in nbstripout - or some other way to break the deadlock so I can use just lf endings on Windows?

[kynan]

Could this be related to #202 ? I attempted to address a Windows-specific line ending issue reported by @varunsh-xilinx in 9f54f83.

This was released in 0.8.2. @owenlamont do you see a different behavior when using 0.8.1 in your pre-commit config?

[owenlamont]

Hi @kynan - yes can confirm if I downgrade to 0.8.1 the problem stops happening.

[owenlamont]

I'm guessing nbstripout just needs a way it can be configured to have compatible behaviour with the mixed-line-ending pre-commit linters config. By default Windows users might like \r\n line endings but since I work across platform but often on Windows so I set it to \n for every platform.

[kynan]

We could certainly make the newline behaviour configurable. Would you be interested in contributing a flag for this?

[owenlamont]

Sure, I can't commit to when as I'm progressing several projects of my own but I can look into this.

[kynan]

@owenlamont FYI, there's an open PR for this at #209. What do you think should be the default behaviour?

[s-weigand]

While working on #209 I realized that the root cause of the pre-commit hook fight is an incorrect noop behaviour.

Currently, any seekable buffer is overwritten unconditionally, whereas the correct behaviour should be an early exit with False if any_change is False.

If EOL normalization should be the job of nbstripout is a different topic, I would say.

[kynan]

@s-weigand Yes, thanks for flagging this! Agree, we shouldn't be overwriting a file if there's no change, however output still needs to be written to stdout.

[s-weigand]

however output still needs to be written to stdout

The git diff tests kicked me hard when I tried the most naive approach, just thinking about inplace file mutation 😅
Always gr8 to have a good test coverage to prevent ppl from messing up 👌