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.

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 travis check for filename spaces #2821

Merged
merged 4 commits into from
Apr 12, 2019
Merged

Add travis check for filename spaces #2821

merged 4 commits into from
Apr 12, 2019

Conversation

paulb777
Copy link
Member

@paulb777 paulb777 commented Apr 12, 2019

Fix #2223

Spaces in filenames cause problems with style.sh, bazel Swift rules, the command line, and potentially other scripts.

By default, ensure that new files with spaces don't get added to the repo.

! -path './Interop/Firebase Component System.md'
)

result=`find . -name "* *" "${options[@]}"`
Copy link
Contributor

Choose a reason for hiding this comment

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

This approach will fail if there are too many files with spaces in them.

It also fails when editors create filenames with spaces, which Xcode, CMake, and CLion do regularly e.g. for temporary testing schemes. Committing this as-is would make local runs of check.sh impossible.

It may be possible to combine this with what git considers ignored to alleviate the second concern. Let me see if I can figure it out.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks. Using only files git knows about also enables reducing the skip list.

! -path '*/Assets.xcassets/*'
! -path '*/Target Support Files'
! -path '*/Local Podspecs'
! -path './Interop/Firebase Component System.md'
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not rename this file rather than grandfather it?

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

Copy link
Contributor

@wilhuff wilhuff left a comment

Choose a reason for hiding this comment

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

LGTM

# Xcode-generated asset files
/Assets.xcassets/ d

# Grandfathered files
Copy link
Contributor

Choose a reason for hiding this comment

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

No files are grandfathered. Consider removing.

@paulb777 paulb777 merged commit 97d4ab1 into master Apr 12, 2019
@paulb777 paulb777 deleted the pb-filename-spaces branch April 12, 2019 20:15
Corrob pushed a commit that referenced this pull request Apr 25, 2019
* Proposed improvements to space checking (#2824)
@firebase firebase locked and limited conversation to collaborators Oct 17, 2019
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.

style.sh does not handle file names with spaces
3 participants