Do not return an error in log, diff and stash show

[carlfriedrich]

fzf has the following exit codes: 0 Normal exit 1 No match 2 Error 130 Interrupted with CTRL-C or ESC

The forgit functions "log", "diff" and "stash show" are designed to display information, not to perform any action. Hence the fzf exit codes 1 and 130 should not be interpreted as errors, they are normal exit behavior. Exclude these error values and return 0 in these cases.

Check list

• [x] I have performed a self-review of my code
• [x] I have commented my code in hard-to-understand areas
• [x] I have made corresponding changes to the documentation

Description

Type of change

• [x] Bug fix
• [ ] New feature
• [ ] Refactor
• [ ] Breaking change
• [ ] Documentation change

Test environment

• Shell
  • [x] bash
  • [x] zsh
  • [x] fish
• OS
  • [x] Linux
  • [ ] Mac OS X
  • [ ] Windows
  • [ ] Others:

[wfxr]

@carlfriedrich Thank you. But I'm not quite sure if this optimization is necessary. And what if git command fail with code 1?

[carlfriedrich]

@carlfriedrich Thank you. But I'm not quite sure if this optimization is necessary. And what if git command fail with code 1?

@wfxr Good point. I will change the PR to only exclude 130, which does not seem to ever be returned by git. The "no match" error (1) shouldn't be returned anyway in these scenarios, so I'm fine with that.

Of course this optimization is not neccessary, forgit works without it. For people who have error codes displayed in their shell prompt, seeing error codes all the time where there actually are no errors is annoying, though (and can be confusing for new forgit users).

[wfxr]

@carlfriedrich Ok I know your point. But considering we are used to got an non-zero exit code after cancelling or interrupting most programs including fzf I still think there is no need to make forgit an exception.

The error code is useful for writing scripts. For the interactive shell, I think it's better to use color to indicate that if an error has occurred and only check the specific error code by echo $? when we really care about what the error is. For the uses who have error codes displayed in their shell prompt they may actually want to know the exactly exit code so not to modify it is also reasonable.

[carlfriedrich]

@carlfriedrich Ok I know your point. But considering we are used to got an non-zero exit code after cancelling or interrupting most programs including fzf I still think there is no need to make forgit an exception.

@wfxr Hmm, I have to disagree on that. While we surely are used to a non-zero exit code when pressing Ctrl-C on some programs, I still find it distracting to get an error (no matter if displayed as a code or just as a coloured prompt) on regularly exiting programs which are purposed to display information.

Consider programs like htop, man or simply less. They all return 0 when regularly quit with q.

Other CLI git clients like tig, lazygit or gitui do so as well. They all even return 0 when quit with Ctrl-C.

So I really think the information displaying functions of forgit should return 0 as well when there is no actual error.

[wfxr]

@carlfriedrich Thanks for the good explanation. Now I agree with you. It's better to return 0 when exit from the information displaying functions.

[carlfriedrich]

@wfxr Great, thanks a lot for your approval!