lazygit icon indicating copy to clipboard operation
lazygit copied to clipboard

Bad config in `.gitconfig` causes wrong error in lazygit

Open mmahmoudian opened this issue 3 years ago • 7 comments

Describe the bug Bad config in .gitconfig would cause wrong error generation by lazygit

To Reproduce Steps to reproduce the behavior:

  1. mess up your ~/.gitconfig (in my case I had a typo in editor) and save it
  2. go to a git folder
  3. run lazygit which throws this error:

    2022/07/11 09:25:57 Git version must be at least 2.0 (i.e. from 2014 onwards). Please upgrade your git version. Alternatively raise an issue at https://github.com/jesseduffield/lazygit/issues for lazygit to be more backwards compatible.

  4. in the same folder try git status and see this error:

    fatal: bad config line 15 in file /home/mehrad/.gitconfig

Expected behavior I expected that lazygit produces an error that points to the correct issue. imho the git error is clear enough and that can be used.

Screenshots Note that the error code of each command is printed at the end of the next prompt, so git's error code is 128 and lazygit is 1

image

Desktop (please complete the following information):

  • OS: Manjaro Linux
  • Lazygit Version: commit=v0.34, build date=2022-04-27T13:28:28Z, build source=binaryRelease, version=0.34, os=linux, arch=amd64 from the "community repo"

Additional context -- not applicable --

mmahmoudian avatar Jul 11 '22 06:07 mmahmoudian

Yeah, currently we throw the same error for whatever git errors out with:

	// if we get an error anywhere here we'll show the same status
	minVersionError := errors.New(app.Tr.MinGitVersionError)

@jesseduffield should we just print out git's error here actually? And the version validation could be done in the same pass, we'd just need to match git version \d.\d+.\d+?

mark2185 avatar Jul 11 '22 07:07 mark2185

Yep if we get an actual error back from git we should just surface that. If we don't get an error back, then I'd parse the output for the version

jesseduffield avatar Jul 11 '22 08:07 jesseduffield

@mmahmoudian would you be interested in fixing this issue? It's okay to say no, don't worry :)

mark2185 avatar Jul 13 '22 07:07 mark2185

@mmahmoudian would you be interested in fixing this issue? It's okay to say no, don't worry :)

I've never done anything in Go. Unless you are really ready to deal with and accept a PR from an absolute Go n00b.

mmahmoudian avatar Jul 13 '22 08:07 mmahmoudian

I've never done anything in Go. Unless you are really ready to deal with and accept a PR from an absolute Go n00b.

Sure! We all gotta start somewhere, what better place to start than lazygit :)

mark2185 avatar Jul 13 '22 09:07 mark2185

I appreciate the leap of faith you are willing to take 🍻 . Then please assign it to me and I'll do my best.

mmahmoudian avatar Jul 13 '22 12:07 mmahmoudian

I think I have to back out of this task as I cannot deliver in timely manner due to lack of enough Go knowledge and also busy schedule. I just add some info here for whoever wants to move forward:

How to reproduce the issue:

There might be many ways to corrupt your ~/.gitconfig, but the one I found the easiest is to open it in [n]vim, navigate to the middle of the name of an item (e.g editor), go to Insert mode (by pressing i), then insert a zero-byte character there by pressing CtrlV and then zero multiple times.

The best approach imo to address this:

imho the best way to approach this is to have a function to check the git viability, something like the following:

func initialGitCheck () error {
	output, err := app.OSCommand.Cmd.New("git --version").RunWithOutput()
	if err == nil {
		return nil
	} else {
		return err
	}
}

Then it should be called at the beginning of isGitVersionValid function to handle the issues with git first, and then move forward. After this, the validateGitVersion should be either removed or merged as part of it is redundant.

mmahmoudian avatar Aug 11 '22 14:08 mmahmoudian