Skip to content

Check git mode also in WSL #512

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

Closed
wants to merge 3 commits into from
Closed

Check git mode also in WSL #512

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c77bff1
Check git mode also in WSL
emontnemery Aug 17, 2020
1f3d56b
Fix format
emontnemery Aug 17, 2020
7002d35
Fix double quotes
emontnemery Aug 17, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion pre_commit_hooks/check_executables_have_shebangs.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
"""Check that executable text files have a shebang."""
import argparse
import platform
import shlex
import sys
from typing import List
Expand All @@ -21,7 +22,9 @@ def zsplit(s: str) -> List[str]:


def check_executables(paths: List[str]) -> int:
if sys.platform == 'win32': # pragma: win32 cover
if (
sys.platform == 'win32' or 'microsoft' in platform.uname()[3].lower()
Copy link
Member

Choose a reason for hiding this comment

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

can you share an example output here and/or add a test? I don't have access to WSL to verify this change

Copy link
Author

Choose a reason for hiding this comment

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

Should I add a comment in the code with the sample output?
I'm not sure what kind of test to add for this, mock the output of platform.uname() and assert _check_git_filemode is called?

Sample output:

$ python3
Python 3.7.5 (default, Nov  7 2019, 10:50:52)
[GCC 8.3.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import platform
>>> platform.uname()
uname_result(system='Linux', node='DESKTOP-A7438J1', release='4.4.0-19041-Microsoft', version='#1-Microsoft Fri Dec 06 14:06:00 PST 2019', machine='x86_64', processor='x86_64')
>>> platform.uname()[3].lower()
'#1-microsoft fri dec 06 14:06:00 pst 2019'
>>>

Copy link
Author

Choose a reason for hiding this comment

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

@asottile Is the sample output enough, and if not, is the suggested test case acceptable?

Copy link
Member

Choose a reason for hiding this comment

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

this check seems much too weak imo

): # pragma: win32 cover
return _check_git_filemode(paths)
else: # pragma: win32 no cover
retv = 0
Expand Down