Skip to content

Conversation

windsok
Copy link
Contributor

@windsok windsok commented May 7, 2021

Couple of minor fixes & improvements for files linter test added in #21740

  • Use a context manager when opening files, so that files are closed are we are done with them

  • Use the -z flag when shelling out to git ls-files so that we can catch newlines and other weird control characters in filenames.

From the git ls-files manpage:

-z \0 line termination on output and do not quote filenames. See OUTPUT below for more information.

Without the -z option, pathnames with "unusual" characters are quoted as explained for the configuration variable
core.quotePath (see git-config(1)). Using -z the filename is output verbatim and the line is terminated by a NUL byte.

Updates the lint-files.py lint test:
* Use a context manager when opening files, so that files are closed.
* Use the -z flag when shelling out to git ls-files so that we can catch newlines
  and other weird control characters in filenames
filename_regex = re.compile(ALLOWED_FILENAME_REGEXP)
failed_tests = 0
for filename in filenames:
if not filename_regex.match(filename):
print(
f"""File "{filename}" does not not match the allowed filename regexp ('{ALLOWED_FILENAME_REGEXP}')."""
f"""File {repr(filename)} does not not match the allowed filename regexp ('{ALLOWED_FILENAME_REGEXP}')."""
Copy link
Contributor Author

@windsok windsok May 7, 2021

Choose a reason for hiding this comment

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

This change is to ensure any unusual characters are printable in the test log.
https://stackoverflow.com/a/31624058
https://docs.python.org/3/library/functions.html#repr

@maflcko
Copy link
Member

maflcko commented May 7, 2021

cr ACK 2227fc4

@practicalswift
Copy link
Contributor

cr ACK 2227fc4: patch looks correct

Thanks for improving linting @windsok. Keep going!

@maflcko maflcko merged commit a33f360 into bitcoin:master May 7, 2021
gwillen pushed a commit to ElementsProject/elements that referenced this pull request Jun 1, 2022
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Aug 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants