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

Head branch #10138

Closed
wants to merge 1 commit into from
Closed

Head branch #10138

wants to merge 1 commit into from

Conversation

luisdavim
Copy link

@luisdavim luisdavim commented Aug 25, 2021

I think this should be more robust since it doesn't rely on a fixed list of potential names.

Standards checklist:

  • The PR title is descriptive.
  • The PR doesn't replicate another PR which is already open.
  • I have read the contribution guide and followed all the instructions.
  • The code follows the code style guide detailed in the wiki.
  • The code is mine or it's from somewhere with an MIT-compatible license.
  • The code is efficient, to the best of my ability, and does not waste computer resources.
  • The code is stable and I have tested it myself, to the best of my abilities.

Changes:

  • Use git branch -rl "*/HEAD" | rev | cut -d/ -f1 | rev to get the main branch instead of trying to guess it.

I think this should be more robust since it doesn't rely on a fixed list of potential names.
@ohmyzsh ohmyzsh bot added Area: plugin Issue or PR related to a plugin Plugin: git labels Aug 25, 2021
fi
done
echo master
git branch -rl "*/HEAD" | rev | cut -d/ -f1 | rev
Copy link
Author

Choose a reason for hiding this comment

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

git symbolic-ref refs/remotes/origin/HEAD | sed 's@^refs/remotes/origin/@@' should also work if you prefer to use sed instead of rev and cut but all of these are pretty standard tools that should be available in any system running git on zsh....

@mcornella
Copy link
Member

See #9879 (comment):

I appreciate what you're trying to do, but we don't need to support all different configurations. The intent behind git_main_branch was to allow the transition from master to another branch name, and it's done that for the most common name alternatives. It doesn't need to do more.

@mcornella mcornella closed this Aug 25, 2021
@luisdavim
Copy link
Author

luisdavim commented Aug 25, 2021

I don't understand how making the function simpler and more generic is worse than covering a specific use case using more complexity.

@mcornella
Copy link
Member

See #9114 on why this doesn't work.

git_main_branch's only purpose is to abstract using master to another standard name. We can't and shouldn't allow non-standard names just because a repository uses it. main and trunk (less often) have been the most popular choices to replace master. Using other names is adding more work, and it needs the reliance on git_main_branch and similar algorithms for user convenience instead of making it easy.

@luisdavim
Copy link
Author

yeah I would understand that if I was adding complexity or some extra specific use case but I'm making the function more generic and reducing the complexity, if the problem is the use of cut and rev we can use some alternatives like cat .git/refs/remotes/origin/HEAD | awk -F '/' '{ print $4 }' or git symbolic-ref refs/remotes/origin/HEAD | sed 's@^refs/remotes/origin/@@'

Why only support the specific case of choosing between main and master when you can support any branch name with a simpler solution?

@luisdavim
Copy link
Author

also, in the current implementation if both main and master exist it will always return main regardless of which is the actual head branch.

@mcornella
Copy link
Member

Please read #9114 again. The remote HEAD was set to develop, so the solution failed to work. Making it more generic makes it worse. You also fail to account for remote-less repositories (go try it on a folder that you just git init).

@mcornella
Copy link
Member

also, in the current implementation if both main and master exist it will always return main regardless of which is the actual head branch.

By design.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: plugin Issue or PR related to a plugin
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants