Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fix _forgit_diff on macOS #375

Merged
merged 1 commit into from
Mar 27, 2024
Merged

Fix _forgit_diff on macOS #375

merged 1 commit into from
Mar 27, 2024

Conversation

sandr01d
Copy link
Collaborator

@sandr01d sandr01d commented Mar 25, 2024

Check list

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

Description

In bash 3.2 uninitialized arrays contain one entry: an empty string, whereas in modern versions of bash they do not contain any entry. I could not find documentation on this but the behavior can be verified by running the following script:

local arr
echo "length=${#arr[@]}"
echo "content=${arr[@]}"

On bash 5.2.26 this prints

length=0
content=

On bash 3.2.57 this prints

length=1
content=

This PR makes sure the $commits and $files arrays are explicitly initialized as empty arrays to allow git to fall back to diffing local changes instead of trying to diff against a revision with a name of an empty string.

Fixes #373

Type of change

  • Bug fix
  • New feature
  • Refactor
  • Breaking change
  • Documentation change

Test environment

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

@sandr01d
Copy link
Collaborator Author

Since this is a regression from #326, I'd like to merge this before the release next week.

@cjappl
Copy link
Collaborator

cjappl commented Mar 26, 2024

This still works with mac and fish 👍

It appears I also have the newer version of bash, so that's why we hadn't seen it before. Thanks for the quick fix.

@keatsfonam
Copy link

FYI this still doesn't work with bash 3.2: #373 (comment)

In bash 3.2 uninitialized arrays contain one entry: an empty string,
whereas in modern versions of bash they do not contain any entry. Make
sure the $commits and $files arrays are explicitly initialized as empty
arrays to allow git to fall back to diffing local changes instead of
trying to diff against a revision with a name of an empty string.
@sandr01d
Copy link
Collaborator Author

Thanks for the hint with the bash docker container @keatsfonam, this helped me track it down. Will keep that in mind for future issues. I got it working in bash 3.2 in the container. Could you give it another try?

@sandr01d sandr01d requested a review from cjappl March 26, 2024 21:23
Copy link
Collaborator

@cjappl cjappl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Passes both fish with newer bash (5.2) and in the docker container for me.

OK to merge once the reporting user says it's all good!

@keatsfonam
Copy link

@sandr01d new fix works for me, thank you

@sandr01d sandr01d merged commit d582f2a into wfxr:master Mar 27, 2024
4 checks passed
@sandr01d sandr01d deleted the fix-gd-macos branch April 5, 2024 18:29
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

git diff fails with bad revision '' on OSX
3 participants