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.

Sign up for GitHub

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

Jump to bottom

Add --no-textconv to git diff calls #2743

Merged
merged 1 commit into from Feb 3, 2023
Merged

Add --no-textconv to git diff calls #2743

merged 1 commit into from Feb 3, 2023

Conversation

adamchainz
Copy link
Contributor

Fixes #2742.

I fixed the one in run.py, fixing the bug. Ithen audited the other git diff calls to consider whether text conversion would be suitable there. Those using --name-only don't need it, since Git skips text conversion in that case. The diff in _run_hooks is for human consumption doesn't need it because it's for the human to read.

asottile
asottile reviewed Feb 2, 2023
View reviewed changes
pre_commit/staged_files_only.py Outdated Show resolved Hide resolved
pre_commit/git.py Outdated Show resolved Hide resolved
pre_commit/commands/run.py Outdated Show resolved Hide resolved
pre_commit/commands/run.py Show resolved Hide resolved
@adamchainz adamchainz force-pushed the issue_2742 branch 2 times, most recently from adf2223 to 6691062 Compare February 2, 2023 18:09
asottile
asottile reviewed Feb 2, 2023
View reviewed changes
tests/commands/run_test.py Outdated

with open('.git/config', 'a') as fp:
fp.write('[diff "empty"]\n')
fp.write('textconv = "python -c print()"\n')
Copy link
Member

Choose a reason for hiding this comment

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

this should use sys.executable (or maybe even bash) so it doesn't fail on systems with only python3

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Switched to sys.executable

Copy link
Member

Choose a reason for hiding this comment

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

eheh I think it might have quoting issues now /o\ we'll see what CI says

Copy link
Member

Choose a reason for hiding this comment

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

I would just use echo probably

Copy link
Contributor Author

Choose a reason for hiding this comment

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

echo is only a shell builtin on my system; printf is binary, but it will require printing its args, which can cause a textconv difference since git diff uses a temporary filename for comparison

I added shlex.quote...

Copy link
Member

Choose a reason for hiding this comment

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

I'm surprised you don't have /bin/echo, it's posix specced iirc

Copy link
Contributor Author

Choose a reason for hiding this comment

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

/bin/echo exists yes, I was just being slow and only checked /usr/bin/.

I have changed the test to use true.

@adamchainz adamchainz force-pushed the issue_2742 branch 3 times, most recently from 06511f3 to 439a59c Compare February 3, 2023 15:55
asottile
asottile approved these changes Feb 3, 2023
View reviewed changes
Copy link
Member

@asottile asottile left a comment

Choose a reason for hiding this comment

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

@asottile asottile merged commit cc7bf96 into pre-commit:main Feb 3, 2023
@adamchainz adamchainz deleted the issue_2742 branch February 3, 2023 18:02
@adamchainz
Copy link
Contributor Author

jaypikay pushed a commit to jaypikay/doxy that referenced this pull request Apr 27, 2023
chore(deps): update dependency pre-commit to v3 (#2)
ddc28c5 
This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [pre-commit](https://github.com/pre-commit/pre-commit) | dev-dependencies | major | `^2.21.0` -> `^3.0.0` |

---

### Release Notes

<details>
<summary>pre-commit/pre-commit</summary>

### [`v3.2.2`](https://github.com/pre-commit/pre-commit/blob/HEAD/CHANGELOG.md#&#8203;322---2023-04-03)

[Compare Source](pre-commit/pre-commit@v3.2.1...v3.2.2)

\==================

##### Fixes

-   Fix support for swift >= 5.8.
    -   [#&#8203;2836](pre-commit/pre-commit#2836) PR by [@&#8203;edelabar](https://github.com/edelabar).
    -   [#&#8203;2835](pre-commit/pre-commit#2835) issue by [@&#8203;kgrobelny-intive](https://github.com/kgrobelny-intive).

### [`v3.2.1`](https://github.com/pre-commit/pre-commit/blob/HEAD/CHANGELOG.md#&#8203;321---2023-03-25)

[Compare Source](pre-commit/pre-commit@v3.2.0...v3.2.1)

\==================

##### Fixes

-   Fix `language_version` for `language: rust` without global `rustup`.
    -   [#&#8203;2823](pre-commit/pre-commit#2823) issue by [@&#8203;daschuer](https://github.com/daschuer).
    -   [#&#8203;2827](pre-commit/pre-commit#2827) PR by [@&#8203;asottile](https://github.com/asottile).

### [`v3.2.0`](https://github.com/pre-commit/pre-commit/blob/HEAD/CHANGELOG.md#&#8203;320---2023-03-17)

[Compare Source](pre-commit/pre-commit@v3.1.1...v3.2.0)

\==================

##### Features

-   Allow `pre-commit`, `pre-push`, and `pre-merge-commit` as `stages`.
    -   [#&#8203;2732](pre-commit/pre-commit#2732) issue by [@&#8203;asottile](https://github.com/asottile).
    -   [#&#8203;2808](pre-commit/pre-commit#2808) PR by [@&#8203;asottile](https://github.com/asottile).
-   Add `pre-rebase` hook support.
    -   [#&#8203;2582](pre-commit/pre-commit#2582) issue by [@&#8203;BrutalSimplicity](https://github.com/BrutalSimplicity).
    -   [#&#8203;2725](pre-commit/pre-commit#2725) PR by [@&#8203;mgaligniana](https://github.com/mgaligniana).

##### Fixes

-   Remove bulky cargo cache from `language: rust` installs.
    -   [#&#8203;2820](pre-commit/pre-commit#2820) PR by [@&#8203;asottile](https://github.com/asottile).

### [`v3.1.1`](https://github.com/pre-commit/pre-commit/blob/HEAD/CHANGELOG.md#&#8203;311---2023-02-27)

[Compare Source](pre-commit/pre-commit@v3.1.0...v3.1.1)

\==================

##### Fixes

-   Fix `rust` with `language_version` and a non-writable host `RUSTUP_HOME`.
    -   [pre-commit-ci/issues#&#8203;173](pre-commit-ci/issues#173) by [@&#8203;Swiftb0y](https://github.com/Swiftb0y).
    -   [#&#8203;2788](pre-commit/pre-commit#2788) by [@&#8203;asottile](https://github.com/asottile).

### [`v3.1.0`](https://github.com/pre-commit/pre-commit/blob/HEAD/CHANGELOG.md#&#8203;310---2023-02-22)

[Compare Source](pre-commit/pre-commit@v3.0.4...v3.1.0)

\==================

##### Fixes

-   Fix `dotnet` for `.sln`-based hooks for dotnet>=7.0.200.
    -   [#&#8203;2763](pre-commit/pre-commit#2763) PR by [@&#8203;m-rsha](https://github.com/m-rsha).
-   Prevent stashing when `diff` fails to execute.
    -   [#&#8203;2774](pre-commit/pre-commit#2774) PR by [@&#8203;asottile](https://github.com/asottile).
    -   [#&#8203;2773](pre-commit/pre-commit#2773) issue by [@&#8203;strubbly](https://github.com/strubbly).
-   Dependencies are no longer sorted in repository key.
    -   [#&#8203;2776](pre-commit/pre-commit#2776) PR by [@&#8203;asottile](https://github.com/asottile).

##### Updating

-   Deprecate `language: python_venv`.  Use `language: python` instead.
    -   [#&#8203;2746](pre-commit/pre-commit#2746) PR by [@&#8203;asottile](https://github.com/asottile).
    -   [#&#8203;2734](pre-commit/pre-commit#2734) issue by [@&#8203;asottile](https://github.com/asottile).

### [`v3.0.4`](https://github.com/pre-commit/pre-commit/blob/HEAD/CHANGELOG.md#&#8203;304---2023-02-03)

[Compare Source](pre-commit/pre-commit@v3.0.3...v3.0.4)

\==================

##### Fixes

-   Fix hook diff detection for files affected by `--textconv`.
    -   [#&#8203;2743](pre-commit/pre-commit#2743) PR by [@&#8203;adamchainz](https://github.com/adamchainz).
    -   [#&#8203;2743](pre-commit/pre-commit#2743) issue by [@&#8203;adamchainz](https://github.com/adamchainz).

### [`v3.0.3`](https://github.com/pre-commit/pre-commit/blob/HEAD/CHANGELOG.md#&#8203;303---2023-02-01)

[Compare Source](pre-commit/pre-commit@v3.0.2...v3.0.3)

\==================

##### Fixes

-   Revert "Prevent local `Gemfile` from interfering with hook execution.".
    -   [#&#8203;2739](pre-commit/pre-commit#2739) issue by [@&#8203;Roguelazer](https://github.com/Roguelazer).
    -   [#&#8203;2740](pre-commit/pre-commit#2740) PR by [@&#8203;asottile](https://github.com/asottile).

### [`v3.0.2`](https://github.com/pre-commit/pre-commit/blob/HEAD/CHANGELOG.md#&#8203;302---2023-01-29)

[Compare Source](pre-commit/pre-commit@v3.0.1...v3.0.2)

\==================

##### Fixes

-   Prevent local `Gemfile` from interfering with hook execution.
    -   [#&#8203;2727](pre-commit/pre-commit#2727) PR by [@&#8203;asottile](https://github.com/asottile).
-   Fix `language: r`, `repo: local` hooks
    -   [pre-commit-ci/issues#&#8203;107](pre-commit-ci/issues#107) by [@&#8203;lorenzwalthert](https://github.com/lorenzwalthert).
    -   [#&#8203;2728](pre-commit/pre-commit#2728) PR by [@&#8203;asottile](https://github.com/asottile).

### [`v3.0.1`](https://github.com/pre-commit/pre-commit/blob/HEAD/CHANGELOG.md#&#8203;301---2023-01-26)

[Compare Source](pre-commit/pre-commit@v3.0.0...v3.0.1)

\==================

##### Fixes

-   Ensure coursier hooks are available offline after install.
    -   [#&#8203;2723](pre-commit/pre-commit#2723) PR by [@&#8203;asottile](https://github.com/asottile).

### [`v3.0.0`](https://github.com/pre-commit/pre-commit/blob/HEAD/CHANGELOG.md#&#8203;300---2023-01-23)

[Compare Source](pre-commit/pre-commit@v2.21.0...v3.0.0)

\==================

##### Features

-   Make `language: golang` bootstrap `go` if not present.
    -   [#&#8203;2651](pre-commit/pre-commit#2651) PR by [@&#8203;taoufik07](https://github.com/taoufik07).
    -   [#&#8203;2649](pre-commit/pre-commit#2649) issue by [@&#8203;taoufik07](https://github.com/taoufik07).
-   `language: coursier` now supports `additional_dependencies` and `repo: local`
    -   [#&#8203;2702](pre-commit/pre-commit#2702) PR by [@&#8203;asottile](https://github.com/asottile).
-   Upgrade `ruby-build` to `20221225`.
    -   [#&#8203;2718](pre-commit/pre-commit#2718) PR by [@&#8203;jalessio](https://github.com/jalessio).

##### Fixes

-   Improve error message for invalid yaml for `pre-commit autoupdate`.
    -   [#&#8203;2686](pre-commit/pre-commit#2686) PR by [@&#8203;asottile](https://github.com/asottile).
    -   [#&#8203;2685](pre-commit/pre-commit#2685) issue by [@&#8203;CarstenGrohmann](https://github.com/CarstenGrohmann).
-   `repo: local` no longer provisions an empty `git` repo.
    -   [#&#8203;2699](pre-commit/pre-commit#2699) PR by [@&#8203;asottile](https://github.com/asottile).

##### Updating

-   Drop support for python<3.8
    -   [#&#8203;2655](pre-commit/pre-commit#2655) PR by [@&#8203;asottile](https://github.com/asottile).
-   Drop support for top-level list, use `pre-commit migrate-config` to update.
    -   [#&#8203;2656](pre-commit/pre-commit#2656) PR by [@&#8203;asottile](https://github.com/asottile).
-   Drop support for `sha` to specify revision, use `pre-commit migrate-config`
    to update.
    -   [#&#8203;2657](pre-commit/pre-commit#2657) PR by [@&#8203;asottile](https://github.com/asottile).
-   Remove `pre-commit-validate-config` and `pre-commit-validate-manifest`, use
    `pre-commit validate-config` and `pre-commit validate-manifest` instead.
    -   [#&#8203;2658](pre-commit/pre-commit#2658) PR by [@&#8203;asottile](https://github.com/asottile).
-   `language: golang` hooks must use `go.mod` to specify dependencies
    -   [#&#8203;2672](pre-commit/pre-commit#2672) PR by [@&#8203;taoufik07](https://github.com/taoufik07).

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box

---

This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNS42MS4wIiwidXBkYXRlZEluVmVyIjoiMzUuNjEuMCJ9-->

Co-authored-by: Renovate Bot <renovate@localhost.localdomain>
Reviewed-on: https://git.goatpr0n.de/public/doxy/pulls/2
Co-authored-by: renovate <renovate@noreply.localhost>
Co-committed-by: renovate <renovate@noreply.localhost>
@ohmer ohmer mentioned this pull request Jul 3, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@asottile asottile asottile approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Binary file change by hook not detected due to Git textconv
2 participants
@adamchainz @asottile