Skip to content

WIP Improve handling of challenging file names when 'core.quotePath=true' #80

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

Merged
merged 1 commit into from
May 21, 2020

Conversation

jmurty
Copy link
Collaborator

@jmurty jmurty commented May 4, 2020

As described in issue #78 transcrypt could fail to properly handle:

  • file names with non-ASCII characters when Git's configuration
    option 'core.quotePath' is set to 'true' (as it is by default)
  • file names containing spaces

This change improves handling of non-ASCII or space characters in
filenames and adds a test to exercise and confirm the fix.

Changes in particular:

  • fix handling of space characters in file name in pre-commit hook
  • override repository's 'core.quotePath' setting in commands that
    output paths to ensure 'core.quotePath=false' is always applied
    even when 'core.quotePath' is explicitly or implicitly (default)
    set to true.

As described in issue #78 transcrypt could fail to properly handle:

- file names with non-ASCII characters when Git's configuration
  option 'core.quotePath' is set to 'true' (as it is by default)
- file names containing spaces

This change improves handling of non-ASCII or space characters in
filenames and adds a test to exercise and confirm the fix.

Changes in particular:

- fix handling of space characters in file name in pre-commit hook
- override repository's 'core.quotePath' setting in commands that
  output paths to ensure 'core.quotePath=false' is always applied
  even when 'core.quotePath' is explicitly or implicitly (default)
  set to true.
Copy link
Owner

@elasticdog elasticdog left a comment

Choose a reason for hiding this comment

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

This looks good to me...consistently quoting things is always difficult in the realm of Bash. It's a bit ugly to have to manually sprinkle -c core.quotePath=false all over the place, but using -z and handling NUL byte characters probably wouldn't be any better.

Feel free to merge this whenever you're comfortable.

@jmurty
Copy link
Collaborator Author

jmurty commented May 12, 2020

Thanks for the feedback @elasticdog. I too am unhappy with having -c options sprinkled through the code. I have been thinking through alternatives that still fix the underlying issue, but haven't come up with any great alternatives.

Options I have considered:

  1. Forcibly set core.quotePath=false in transcrypt-enabled repositories. This would solve the problem, but in a somewhat aggressive way which might break peoples workflows or tooling. And it seems like a bad idea to override a Git default value like that unless it's really needed.

  2. Replace the sprinkled -c overrides with Git aliases which versions that include the -c core.quotePath=false override, e.g: ls-files-unquote, check-attr-unquote, ls-tree-unquote. The transcrypt script would then use these alternatives.

    This is cleaner since it sets the overrides in one place, and works okay in my initial testing except it breaks usage of transcrypt in un-inited repos for actions like transcrypt --list.

@jmurty jmurty merged commit b5af188 into master May 21, 2020
@jmurty jmurty deleted the improve-handling-of-challenging-file-names branch May 22, 2020 06:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants