AGWA / git-crypt

Transparent file encryption in git
https://www.agwa.name/projects/git-crypt/
GNU General Public License v3.0
8.32k stars 479 forks source link

Make the repo state directory location configurable. #86

Closed nirvdrum closed 8 years ago

nirvdrum commented 8 years ago

This PR implements a configuration for the repo state directory location, following from a discussion on the mailing list.

I'm opening early in hopes of getting any feedback. It's been a while since I've done C++ and if I should be adjusting anything, that would be good to know. In particular, I'm not sure if my check for leading '/' in the path is idiomatic C++ and/or if it works in the presence of paths with non-Latin characters.

There is a potential race between the check if a config item exists and its retrieval, but it should be safe because the actual retrieval will throw an exception if the key was deleted in the interim. If you'd prefer, I can alter the code to just run the "get" operation and handle the exception rather than having a leading "ask" operation.

Finally, this should be 100% backwards-compatible. It seems to work in every scenario I've tried, but I'll run through some more.

AGWA commented 8 years ago

Thanks! I think the potential race is fine and I agree this will be backwards-compatible. A couple things I'd like to see changed:

  1. Move the std::stringstream and std::getline logic into get_git_config and have it return a std::string. This encapsulates the logic better.
  2. Drop the check for the leading / and unconditionally append a /. This might cause a double / character sometimes but that's OK.
nirvdrum commented 8 years ago

Thanks for the feedback. For whatever reason I thought you were favoring [out] params. Looking back, that's only the case for methods that return status codes. I've pushed that and renamed the key param to name to better match the git_has_config function.

nirvdrum commented 8 years ago

As for documentation, would you like a blurb in the README or a wiki page?

nirvdrum commented 8 years ago

This seems to be working well for me. Is there anything I need to do to get it over the finish line?

AGWA commented 8 years ago

Thanks for reminding me about this! I just cherry-picked as 788a6a99f4289745e6bd12fae2ad8014af320a4f.