-
Notifications
You must be signed in to change notification settings - Fork 30
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
Use git environment variables if set #42
Conversation
Thank you for catching it. I didn't know |
autoload/committia/git.vim
Outdated
" Use environment variables if set | ||
if !empty($GIT_DIR) && !empty($GIT_WORK_TREE) | ||
return [$GIT_DIR, $GIT_WORK_TREE] | ||
endif |
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.
$GIT_WORK_TREE
is set by user and the path must be existing. So $GIT_WORK_TREE
should be verified as follows:
if !empty($GIT_DIR) && !empty($GIT_WORK_TREE) && isdirectory($GIT_WORK_TREE)
return [$GIT_DIR, $GIT_WORK_TREE]
endif
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.
Or should we explicitly raise an error rather than falling back into automatic worktree detection silently?
if !empty($GIT_DIR) && !empty($GIT_WORK_TREE)
if !isdirectory($GIT_WORK_TREE)
throw 'Directory specified with $GIT_WORK_TREE is not found: ' . $GIT_WORK_TREE
endif
return [$GIT_DIR, $GIT_WORK_TREE]
endif
Either look ok for me.
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.
Good point, I set GIT_WORK_TREE
manually, that's correct. I like the variant throwing an error because git cannot operate if the value of GIT_WORK_TREE
is invalid:
✔ test master@HEAD$ GIT_WORK_TREE="/tmp/invalid" git status
fatal: This operation must be run in a work tree
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.
Looks good. Thank you.
Git also uses the
GIT_DIR
andGIT_WORK_TREE
environment variables , which should be preferred if set.This does not fix #37 yet, but provides a base for vcsh to work. Vcsh only has to export those environment variables and it will work.
In the meantime, other scripts relying on those variables will work right away.