-
-
Notifications
You must be signed in to change notification settings - Fork 718
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
Add check for illegal Widnows names #590
Add check for illegal Widnows names #590
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
actually, crazy idea -- I think we can replace this entire hook with a fail
hook without needing to write any code:
- id: check-illegal-windows-names
name: check illegal windows names
entry: Illegal windows filenames detected
language: fail
files: '(?i)(^|/)(CON|PRN|AUX|NUL|COM[1-9]|LPT[1-9])(\.|/|$)'
from typing import Sequence | ||
from typing import Set | ||
|
||
from pre_commit_hooks.util import added_files |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it should just filter all files, instead of using added_files
here
while '.' in root: | ||
root, _ = os.path.splitext(root) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this can be better written as root, _, _ = root.partition('.')
|
||
|
||
# https://docs.microsoft.com/en-us/windows/win32/fileio/naming-a-file | ||
ILLEGAL_NAMES = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
frozenset((
while '.' in root: | ||
root, _ = os.path.splitext(root) | ||
if root.lower() in lower_set(ILLEGAL_NAMES): | ||
print(f'Illegal name {filename}') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Illegal windows filename: {filename}
'filenames', nargs='*', | ||
help='Filenames pre-commit believes are changed.', | ||
) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can remove this newline
|
||
args = parser.parse_args(argv) | ||
|
||
return find_illegal_windows_names(args.filenames) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this can probably be inlined or underscored -- I don't think having a separate function improves anything here (especially once you remove the added_files
bit)
skip_win32 = pytest.mark.skipif( | ||
sys.platform == 'win32', | ||
reason='case conflicts between directories and files', | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think once you remove added_files
as a constraint you won't need to skip anything (you won't even need to have any disk state either!)
Fixes #589