Bash/zsh function that cd to the root of git tree

Question

I'd like to get a review for the function that cd into the git tree root or does nothing if we are outside of the repository.

reviewed version:

# cd to the root of the current git directory
# If $PWD is submodule, will cd to the root of the top ancestor
# It requires to stay in the current directory, if the root is . or unknown,
# and use cd only once, to have a way to do `cd -`
function cg {
 git_root() {
 local super_root
 local top
 top="$(git rev-parse --show-cdup)"
 top="${top:-./}"
 super_root="$(git rev-parse --show-superproject-working-tree)"
 if [[ "$super_root" ]]; then
 printf '%s' "$top../"
 ( cd "$top../" && git_root || return )
 fi
 printf '%s' "$top"
 }
 local git_root
 git_root="$(git_root)"
 [ "x${git_root}" != "x./" ] && cd "$(git_root)" && return || return 0
}

updated version:

#!/bin/bash
# cd to the root of the current git directory
# If $PWD is submodule, will cd to the root of the top ancestor
# It requires to stay in the current directory, if the root is . or unknown,
# and use cd only once, to have a way to do `cd -`
function cg {
 function git_root {
 local top; top="$(git rev-parse --show-cdup)"
 top="${top:-./}"
 local super_root; super_root="$(git rev-parse --show-superproject-working-tree)"
 if [[ "$super_root" ]]; then
 printf '%s' "$top../"
 ( cd "$top../" && git_root || return )
 fi
 printf '%s' "$top"
 }
 local tree_root
 tree_root="$(git_root)"
 [[ "x${tree_root}" != "x./" ]] && cd "${tree_root}" && return || return 0
}

• hjpotter92: just a suggestion, the single line cd "$(git rev-parse --show-toplevel || echo .)" does the exact thing :) Taken from oh-my-zsh plugin: github.com/ohmyzsh/ohmyzsh/blob/master/plugins/git/…
• pacmaninbw: Please do not edit the question after it has been answered. See what should I not do.
• Felixoid: @hjpotter92 no, sorry, it doesn't because it knows nothing about: a) symlinks; b) submodules. These two points forced me to invest time in the function above.
• Felixoid: Thank you pacmaninbw, here's the updated version codereview.stackexchange.com/questions/252838/…

Answer 1

Great

• 100% pass on shellcheck which means you're doing great with quoting everything that could be potentially problematic.
• scoping variables
• using [[ conditional for the if
• nice explanation of what it is doing and why

Could be better

• a sh-bang line at the top is a good idea for scripts, even if this would usually just be sourced during your login scripts.
• Your inner function is defined with different syntax than your outer function.
• You can scope and define a variable in one line. For example: local super_root="$(git rev-parse --show-superproject-working-tree)". This makes sure you don't fail to define a local variable or fail to scope a new variable. And it cuts out a line of code in each case.
• Reusing git_root for a variable name and the function name is confusing. I initially wondered why you were trying to scope the function after you had just defined it.
• use [[ conditional for conditional in the last line.

• Toby Speight: I know it's tagged bash, but I'd go the other way on your last comment, and turn the existing [[ into a [ test, so this function can more easily be adapted for other shells (I think that just leaves local as the only Bash feature used).
• Felixoid: Thank you for your review!
• Felixoid: 1) sure, makes sense; 2) true, I've missread the name() ( code ) and name() { code } notations; 3) actually making one line local top=... cause complaints by shellcheck SC2155: Declare and assign separately to avoid masking return values. That's why I've decided to leave it this way; 4) yes, a little bit messy; 5) I see the point to make it POSIX-complaint, but this looks to me like a function for interactive shell, so I'll use [[ in the latest line too.
• chicks: Thanks for reviewing the review! I like [[ for safety, but I'm not worried about portability like Toby. SC2155 applies because all your assignments involve subcommands, but you're already masking return values in a few places.
• chicks: I like the cleaned up version, but to get a new review you're supposed to post a new question each time.