Re: [PATCH 1/2] git-help: add "help.format" config variable.

Junio C Hamano [off-list ref] writes:

Christian Couder [off-list ref] writes:

quoted

diff --git a/git.c b/git.c
index 4f9876e..d46b63d 100644
--- a/git.c
+++ b/git.c

@@ -324,7 +324,7 @@ static void handle_internal_command(int argc, const char **argv)
 		{ "gc", cmd_gc, RUN_SETUP },
 		{ "get-tar-commit-id", cmd_get_tar_commit_id },
 		{ "grep", cmd_grep, RUN_SETUP | USE_PAGER },
-		{ "help", cmd_help },
+		{ "help", cmd_help, RUN_SETUP },
 #ifndef NO_CURL
 		{ "http-fetch", cmd_http_fetch, RUN_SETUP },
 #endif

It would be _NICE_ if we read configuration when we are in a git
repository, but I am afraid this change is unnice -- the users used to
be able to say "git help" from anywhere didn't they?  Now they will get
"Not a git repository".  It needs to do an optional repository
discovery, not a mandatory one RUN_SETUP causes.

It turns out that the earlier git-browse-help is already broken with
respect to this.

-- >8 --
[PATCH] git-help -w: do not require to be in git repository

The users used to be able to say "git help cat-file" from anywhere, but
the browse-help script insisted to be in a git repository, which caused
"git help -w cat-file" to barf outside.  Correct it.

While at it, remove leftover debugging "echo".

Signed-off-by: Junio C Hamano <redacted>
---
 git-browse-help.sh |   13 ++++++++-----
 git-sh-setup.sh    |   45 ++++++++++++++++++++++++++-------------------
 2 files changed, 34 insertions(+), 24 deletions(-)

diff --git a/git-browse-help.sh b/git-browse-help.sh
index 817b379..b465911 100755
--- a/git-browse-help.sh
+++ b/git-browse-help.sh

@@ -17,8 +17,10 @@
 #
 
 USAGE='[--browser=browser|--tool=browser] [cmd to display] ...'
-SUBDIRECTORY_OK=Yes
-OPTIONS_SPEC=
+
+# This must be capable of running outside of git directory, so
+# the vanilla git-sh-setup should not be used.
+NONGIT_OK=Yes
 . git-sh-setup
 
 # Install data.

@@ -37,7 +39,7 @@ valid_tool() {
 }
 
 init_browser_path() {
-	browser_path=`git config browser.$1.path`
+	test -z "$GIT_DIR" || browser_path=`git config browser.$1.path`
 	test -z "$browser_path" && browser_path=$1
 }
 

@@ -69,7 +71,8 @@ do
     shift
 done
 
-if test -z "$browser"; then
+if test -z "$browser" && test -n "$GIT_DIR"
+then
     for opt in "help.browser" "web.browser"
     do
 	browser="`git config $opt`"

@@ -91,7 +94,7 @@ if test -z "$browser" ; then
     else
 	browser_candidates="w3m links lynx"
     fi
-    echo "browser candidates: $browser_candidates"
+
     for i in $browser_candidates; do
 	init_browser_path $i
 	if type "$browser_path" > /dev/null 2>&1; then

diff --git a/git-sh-setup.sh b/git-sh-setup.sh
index 5aa62dd..b366761 100755
--- a/git-sh-setup.sh
+++ b/git-sh-setup.sh

@@ -122,26 +122,33 @@ get_author_ident_from_commit () {
 	LANG=C LC_ALL=C sed -ne "$pick_author_script"
 }
 
-# Make sure we are in a valid repository of a vintage we understand.
-if [ -z "$SUBDIRECTORY_OK" ]
+# Make sure we are in a valid repository of a vintage we understand,
+# if we require to be in a git repository.
+if test -n "$NONGIT_OK"
 then
-	: ${GIT_DIR=.git}
-	test -z "$(git rev-parse --show-cdup)" || {
-		exit=$?
-		echo >&2 "You need to run this command from the toplevel of the working tree."
-		exit $exit
-	}
+	if git rev-parse --git-dir >/dev/null 2>&1
+	then
+		: ${GIT_DIR=.git}
+	fi
 else
-	GIT_DIR=$(git rev-parse --git-dir) || {
-	    exit=$?
-	    echo >&2 "Failed to find a valid git directory."
-	    exit $exit
+	if [ -z "$SUBDIRECTORY_OK" ]
+	then
+		: ${GIT_DIR=.git}
+		test -z "$(git rev-parse --show-cdup)" || {
+			exit=$?
+			echo >&2 "You need to run this command from the toplevel of the working tree."
+			exit $exit
+		}
+	else
+		GIT_DIR=$(git rev-parse --git-dir) || {
+		    exit=$?
+		    echo >&2 "Failed to find a valid git directory."
+		    exit $exit
+		}
+	fi
+	test -n "$GIT_DIR" && GIT_DIR=$(cd "$GIT_DIR" && pwd) || {
+		echo >&2 "Unable to determine absolute path of git directory"
+		exit 1
 	}
+	: ${GIT_OBJECT_DIRECTORY="$GIT_DIR/objects"}
 fi
-
-test -n "$GIT_DIR" && GIT_DIR=$(cd "$GIT_DIR" && pwd) || {
-    echo >&2 "Unable to determine absolute path of git directory"
-    exit 1
-}
-
-: ${GIT_OBJECT_DIRECTORY="$GIT_DIR/objects"}

-- 
1.5.3.7-1170-g8d08

Le jeudi 13 décembre 2007, Junio C Hamano a écrit :

Junio C Hamano [off-list ref] writes:

quoted

Christian Couder [off-list ref] writes:

quoted

diff --git a/git.c b/git.c
index 4f9876e..d46b63d 100644
--- a/git.c
+++ b/git.c

@@ -324,7 +324,7 @@ static void handle_internal_command(int argc,

const char **argv) { "gc", cmd_gc, RUN_SETUP },
 		{ "get-tar-commit-id", cmd_get_tar_commit_id },
 		{ "grep", cmd_grep, RUN_SETUP | USE_PAGER },
-		{ "help", cmd_help },
+		{ "help", cmd_help, RUN_SETUP },
 #ifndef NO_CURL
 		{ "http-fetch", cmd_http_fetch, RUN_SETUP },
 #endif

It would be _NICE_ if we read configuration when we are in a git
repository, but I am afraid this change is unnice -- the users used to
be able to say "git help" from anywhere didn't they?  Now they will get
"Not a git repository".  It needs to do an optional repository
discovery, not a mandatory one RUN_SETUP causes.

It turns out that the earlier git-browse-help is already broken with
respect to this.

You are right, I did not test outside a git repository.
I reworked the patch and will send it after this email.

While testing my new patch it seemed to me that some of your changes in the 
patch quoted below prevent some configuration variables to be used when 
they are set in the "global" config file (~/.gitconfig). So I reverted them 
in my new patch. But thanks to your other changes, it seems to work fine 
now.

-- >8 --
[PATCH] git-help -w: do not require to be in git repository

[...]

This seems to prevent using global configuration when outside a git repo.

This also.

Thanks,
Christian.