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

Feature/full zsh feature #17

Closed

Conversation

Ondkloss
Copy link

@Ondkloss Ondkloss commented Mar 17, 2022

Attempt at a full inclusion of the changes made in my fork into this base repo.
I will let this sink in a bit (do a visual review), but essentially this:

  • Adds zsh support
  • Touches .bash_profile and .zshrc to ensure existance
  • Minor changes to README.md

This PR is intended to replace my previous one, as it was a bare minimum zsh inclusion, while this is all the changes.
If this goes through I intend to alert and/or archive my fork for clarity.

@Ondkloss
Copy link
Author

Ok, gave it a second sanity check and it seems good. If I don't hear any input (in the negative direction) from @demedos and @fabriziocucci I'll go ahead and squash merge this in. It should resolve ALL currently open PRs as the issues in the other ones are either merged or otherwise covered by this PR.

Copy link
Owner

@fabriziocucci fabriziocucci left a comment

Choose a reason for hiding this comment

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

My two cents on the matter if supporting zsh is really something we want to do here:

  1. the name of the repo should probably be more generic (e.g. git-shell-for-mac)
  2. .git-bash-for-mac-zsh.sh should probably be named git-zsh-for-mac.sh to avoid confusion
  3. the install/uninstall script should detect if it the default shell of the host running it is zsh (latest macOS versions) or bash (for the few aficionados) and act accordingly

Overall, I'm starting to think it's probably best to simply create a separate repo for zsh only and reference that from the README in this repo. I believe it would just make things more clear. Of course @Ondkloss, I would suggest you to be the owner of such repo since, as I mentioned, I'm no longer using this script.

@Ondkloss
Copy link
Author

Regarding your answer @fabriziocucci:

  1. I think the naming is quite relevant for SEO though, because I think most come from the mindset of having used git-bash on Windows and want the same for Mac. If you name it git-shell-for-mac or even git-zsh-for-mac the link might be broken.
  2. Would agree.
  3. There is some case for someone perhaps using multiple and attaching to both being best, but you could do default or maybe even an option for "bash, zsh or both" during install.

I think your reply is completely reasonable, and with it in mind after all this time I think perhaps we might just go with the status quo. I don't think I'll create a new repo, as opposed to just keeping my fork as the place for modifications that include zsh, and perhaps rather do your suggested improvements there instead.

That way this repo stays clean on bash while my fork solves issues for those who want zsh and perhaps some more.

@demedos
Copy link

demedos commented Mar 18, 2022

As @Ondkloss said, most people will probably find this repository looking for a Mac alternative to Git Bash for Windows, thus it would be a good idea to merge those changes in

@fabriziocucci
Copy link
Owner

@Ondkloss

  1. I thought about it as well yesterday and what you are saying makes totally sense, although if I look back at when I created this repo, I didn't really pick the name for SEO reasons but only because the current name represents exactly what the project is i.e. the equivalent for mac of the Git Bash for Windows. This is why I'd rather keep a name which is technically correct and simply mention in the README your fork for zsh support or more heavyweight solutions like Oh My Zsh.

  2. Same here, the thought crossed my mind yesterday but I believe that is really a corner case and, I would argue, if a user is switching between bash, zsh and other shells, he/she probably knows what is doing i.e. we are probably talking about a more savvy user compared to a mac newbie which was the original target of this repo.

@demedos

As mentioned before, being technically correct/consistent and having the zsh support only one click away (from the README of this repo) doesn't sound really bad, or does it?

@Ondkloss Ondkloss mentioned this pull request Mar 21, 2022
@Ondkloss
Copy link
Author

I suggest the following plan of action on PR/issues:

At the same time:

  • Create a new PR pointing to my fork for zsh-support as well as pointing to "Oh My Zsh" for powerusers.

Unless you object @fabriziocucci or @demedos, I'll probably execute in the coming days.

@demedos
Copy link

demedos commented Mar 22, 2022

Sounds good to me

@Ondkloss
Copy link
Author

I see that my available options (without write access) are rather limited. I'm executing the actions that I have available and I'll leave the rest up to either those who own the issue/PR, or someone with write access to the repo.

With this I'm closing off this PR and moving to my fork being the go to destination for zsh support.

@Ondkloss Ondkloss closed this Mar 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants