davidraviv / gh-clean-branches

Safely deletes local branches with no upstream and no un-pushed commits
MIT License
177 stars 15 forks source link

fix: refactor to bash compatibility #18

Open ndom91 opened 2 years ago

ndom91 commented 2 years ago

Why did you make this change

Make it bash compatible and further the reach of your great extension!

Feel free to close if you're a big zsh fan and explicitly want to keep it this way.. Otherwise I think it'd be wise to make this more widely compatible! :tada:

What have you done in this PR

What was left to do

Dependencies

- zsh

Checklist:

waldyrious commented 2 years ago

Awesome work, @ndom91! :clap::clap: For the record, this was first raised in #2, and initial POSIX-ifying work was done in #3.

ndom91 commented 2 years ago

Awesome work, @ndom91! :clap::clap: For the record, this was first raised in #2, and initial POSIX-ifying work was done in #3.

Ah I hadn't even seen those haha. Thanks for the initial work then!!

davidraviv commented 2 years ago

Thanks for the great contribution, @ndom91! I believe that relying on bash will allow more developers to enjoy this extension.

This is a significant change. It will take some time to review and test it.

I have one concern, though. This change may break the extension for a developer who only has zsh installed. Though, I assume that it is a rare case.

ndom91 commented 2 years ago

Thanks for the great contribution, @ndom91! I believe that relying on bash will allow more developers to enjoy this extension.

This is a significant change. It will take some time to review and test it.

I have one concern, though. This change may break the extension for a developer who only has zsh installed. Though, I assume that it is a rare case.

Yeah I can double check and run it with Zsh as well. I have a macbook somewhere.. haha.

EDIT: Can confirm the array splitting doesn't work in zsh now.

image

See:

https://github.com/davidraviv/gh-clean-branches/blob/4b8efdc7cbc62c1f24c1813241c8f97e3a0bc710/gh-clean-branches#L43-L46

https://github.com/davidraviv/gh-clean-branches/blob/4b8efdc7cbc62c1f24c1813241c8f97e3a0bc710/gh-clean-branches#L82

Will work on this shortly

EDIT 2: Note - https://unix.stackexchange.com/a/491459

davidraviv commented 2 years ago

@ndom91 Sorry for the delay in testing this.

Please take a look at a test I made that compares the results of the current code (1st run) to the PR code (2nd run).

The branch add-git-ignore exists both on the remote and local. Hence the branch is not expected to be deleted.

✅ The current code doesn't list the branch for deletion - as expected. ❌ The PR code lists the branch for deletion - not as expected.

You are welcome to take a look and push a fix. I promise to test it quickly 😉

Screen Shot 2022-05-23 at 17 46 39