| Message ID | 4e1a569c9fa494fa9bb1bfde14d77ac9751d9049.1623316412.git.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Fix git subtree on Windows | expand |
On Thu, 10 Jun 2021 03:13:31 -0600, Johannes Schindelin via GitGitGadget wrote: > > From: Johannes Schindelin <johannes.schindelin@gmx.de> > > On Windows, both forward and backslash are valid separators. In > 22d550749361 (subtree: don't fuss with PATH, 2021-04-27), however, we > added code that assumes that it can only be the forward slash. > > Let's fix that. > > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> > --- > contrib/subtree/git-subtree.sh | 8 +++++--- > 1 file changed, 5 insertions(+), 3 deletions(-) > > diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh > index 6bd689a6bb92..d11ac56f9eb8 100755 > --- a/contrib/subtree/git-subtree.sh > +++ b/contrib/subtree/git-subtree.sh > @@ -13,6 +13,8 @@ if test -z "$GIT_EXEC_PATH" || { > } > } || ! test -f "$GIT_EXEC_PATH/git-sh-setup" > then > + base=${0##*/} > + base=${base##*\\} This might be more clearly written as base=${0##*[/\\]} ? (And then maybe it doesn't need a separate variable, and can still be written in-line?)
Hi Luke, On Thu, 10 Jun 2021, Luke Shumaker wrote: > On Thu, 10 Jun 2021 03:13:31 -0600, > Johannes Schindelin via GitGitGadget wrote: > > > > From: Johannes Schindelin <johannes.schindelin@gmx.de> > > > > On Windows, both forward and backslash are valid separators. In > > 22d550749361 (subtree: don't fuss with PATH, 2021-04-27), however, we > > added code that assumes that it can only be the forward slash. > > > > Let's fix that. > > > > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> > > --- > > contrib/subtree/git-subtree.sh | 8 +++++--- > > 1 file changed, 5 insertions(+), 3 deletions(-) > > > > diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh > > index 6bd689a6bb92..d11ac56f9eb8 100755 > > --- a/contrib/subtree/git-subtree.sh > > +++ b/contrib/subtree/git-subtree.sh > > @@ -13,6 +13,8 @@ if test -z "$GIT_EXEC_PATH" || { > > } > > } || ! test -f "$GIT_EXEC_PATH/git-sh-setup" > > then > > + base=${0##*/} > > + base=${base##*\\} > > This might be more clearly written as > > base=${0##*[/\\]} > > ? True. For some reason, this did not work in my tests (probably because I confused `#` with `%` or put the wildcard on the wrong side, or something). Will fix. > (And then maybe it doesn't need a separate variable, and can still be > written in-line?) I am not in favor of inlining here. Three times. It is too complicated a construct for a casual contributor. At least giving it a name (where "basename" would make even more sense than "base", I just realized) alleviates this problem _somewhat_. Ciao, Dscho
diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh index 6bd689a6bb92..d11ac56f9eb8 100755 --- a/contrib/subtree/git-subtree.sh +++ b/contrib/subtree/git-subtree.sh @@ -13,6 +13,8 @@ if test -z "$GIT_EXEC_PATH" || { } } || ! test -f "$GIT_EXEC_PATH/git-sh-setup" then + base=${0##*/} + base=${base##*\\} echo >&2 'It looks like either your git installation or your' echo >&2 'git-subtree installation is broken.' echo >&2 @@ -20,10 +22,10 @@ then echo >&2 " - If \`git --exec-path\` does not print the correct path to" echo >&2 " your git install directory, then set the GIT_EXEC_PATH" echo >&2 " environment variable to the correct directory." - echo >&2 " - Make sure that your \`${0##*/}\` file is either in your" + echo >&2 " - Make sure that your \`$base\` file is either in your" echo >&2 " PATH or in your git exec path (\`$(git --exec-path)\`)." - echo >&2 " - You should run git-subtree as \`git ${0##*/git-}\`," - echo >&2 " not as \`${0##*/}\`." >&2 + echo >&2 " - You should run git-subtree as \`git ${base#git-}\`," + echo >&2 " not as \`$base\`." >&2 exit 126 fi