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

Fix fatal error if not in git dir #109

Merged
merged 1 commit into from
Oct 4, 2017

Conversation

cy4n
Copy link
Contributor

@cy4n cy4n commented May 10, 2017

following command throws an error in git 2.13.0 macosx:

git config --local --get oh-my-git.enabled
fatal: BUG: setup_git_env called without repository
(appears in base.sh:49)

this fix pipes the "fatal..." output to /dev/null (as you already do in every other git command :-)

@UFOMelkor
Copy link

Same bug occurs on openSUSE with git 2.13.0.

@cy4n
Copy link
Contributor Author

cy4n commented May 11, 2017

looks like its not an actual bug, but git added some error messages when you use git in non-git folders
technically this is not a bug in ohmygit either, but you probably want to hide the message :-)

@djbclark
Copy link

Thanks I was seeing this too, linuxbrew after update to 2.13.0. Please merge :-)

#110 is a dupe of this bug.

@borja
Copy link

borja commented May 30, 2017

Tested locally.. Thanks for the fix, @cy4n . LGTM.
Please merge 😄

@huxi
Copy link

huxi commented Jun 26, 2017

Message changed from fatal: BUG: setup_git_env called without repository (2.13.1) to fatal: --local can only be used inside a git repository (2.13.2).

@huxi
Copy link

huxi commented Jun 26, 2017

This should really remove --local instead of piping the error to /dev/null as @sw0x2A said in a #110 comment.

@cy4n
Copy link
Contributor Author

cy4n commented Jun 27, 2017

actually skipping --local would also allow to globally disable oh-my-git by adding

[oh-my-git]
enabled=false

to the global ~/.gitconfig
and only enable it on repos you actually want to use it by adding

[oh-my-git]
enabled=true 

in the local .git/config
this would also prevent the local exploit in #104.
or at least you actually see the branch name in the config when you add the enable=false line

@cy4n
Copy link
Contributor Author

cy4n commented Jun 27, 2017

approved @huxi ;)

param is not needed and provides an error message if called from non-git
folder
@andrewkew-aft
Copy link

When will this PR be merged into master? I would rather take the changes from master than be running a side branch.

@cy4n
Copy link
Contributor Author

cy4n commented Jul 7, 2017

github says that @arialdomartini is still active,) ask him.
this repo has had its last change over a year ago.

@andrewkew-aft
Copy link

Cheers @cy4n I have pinged him, hopefully we can get this approved and merged.

@snowblink
Copy link

Any news on this @arialdomartini ?

rcmachado pushed a commit to rcmachado/oh-my-git that referenced this pull request Jul 29, 2017
Refers to PR arialdomartini#109.

* cyan4/fix_enabled:
  remove --local param
@MindRave
Copy link

Hello! Are there any news on this? @arialdomartini

@arialdomartini arialdomartini self-assigned this Oct 4, 2017
@arialdomartini arialdomartini self-requested a review October 4, 2017 12:28
@arialdomartini arialdomartini removed their assignment Oct 4, 2017
@arialdomartini arialdomartini added this to the v1.6 milestone Oct 4, 2017
arialdomartini added a commit that referenced this pull request Oct 4, 2017
@arialdomartini arialdomartini merged commit a9fefe3 into arialdomartini:master Oct 4, 2017
@arialdomartini
Copy link
Owner

@cy4n, @UFOMelkor, @djbclark, @borja, @huxi, @andrewkew-aft, @snowblink, @MindRave

shame on me. I've been neglecting this repo for more than 1 year already. I've got a bunch of issues to fix and pull requests to merge. Time to get up and work.
Thank you for your job!

@cy4n cy4n deleted the fix_enabled branch October 4, 2017 14:04
@cy4n
Copy link
Contributor Author

cy4n commented Oct 4, 2017

@arialdomartini before you merge all this PRs, maybe checkout my or @compholios fork, because we have picked some of the PRs and received PRs on top of them that are not necessarily in your upstream repo

@cy4n
Copy link
Contributor Author

cy4n commented Oct 4, 2017

anyway welcome back

@arialdomartini
Copy link
Owner

Hi @cy4n.
Sure, thank you!
I think we have 2 options, here. Either I could just merge the last commit of your fork, review all the job you did during the last months, and start over from there; or I could start from the PRs to this repo, always comparing the result with the commits in your repo. In both cases, I think it would be beneficial to reduce as much as possible the distance between this repo and your fork.

I think I will go with the former. Keep in touch, please, I'm going to ask your opinion on some PRs...

Grazie mille!

masukomi added a commit to masukomi/oh-my-git that referenced this pull request Jun 14, 2019
upgrade in git changed viability of --local

fix found via upstream commit
arialdomartini#109 (comment)
DaneWeber added a commit to DaneWeber/oh-my-git that referenced this pull request Oct 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants