[TopGit] Improve help output

[TopGit] Improve help output

[martin f. krafft]

The following patches improve the help output by TopGit. They are
not all related, but they conflict.

I would just push them to the master branch, but I don't want to
risk my chances of Petr getting angry with his new release
assistant...

[TopGit PATCH] Check for help invocation before setup

[martin f. krafft]

The user ought to be able to call `tg help` from anywhere in the
filesystem, not just Git repositories, so the help parsing has to happen
before the calls to git git binary.

Debian bug: #501982

Signed-off-by: martin f. krafft <madduck@debian.org>
---
 tg.sh |   17 ++++++++++++++---
 1 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/tg.sh b/tg.sh
index 2961106..40c4ab7 100644
--- a/tg.sh
+++ b/tg.sh
@@ -235,6 +235,20 @@ do_help()
 	fi
 }
 
+## Startup
+
+args_saved="$@"
+while [ -n "$1" ]; do
+	case "$1" in
+	help|--help|-h)
+		shift
+		do_help "$1"
+		exit 1;;
+	esac
+	shift
+done
+set -- $args_saved
+unset args_saved
 
 ## Initial setup
 
@@ -268,9 +282,6 @@ cmd="$1"
 shift
 
 case "$cmd" in
-help|--help|-h)
-	do_help "$1"
-	exit 1;;
 --hooks-path)
 	# Internal command
 	echo "@hooksdir@";;
-- 
1.6.0.2

[TopGit PATCH] Change tg help exit code to 0

[martin f. krafft]

Printing --help is not an error, but a successful operation, if the help
output could be printed.

Signed-off-by: martin f. krafft <madduck@debian.org>
---
 tg.sh |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/tg.sh b/tg.sh
index 40c4ab7..ea22544 100644
--- a/tg.sh
+++ b/tg.sh
@@ -243,7 +243,7 @@ while [ -n "$1" ]; do
 	help|--help|-h)
 		shift
 		do_help "$1"
-		exit 1;;
+		exit 0;;
 	esac
 	shift
 done
-- 
1.6.0.2

[TopGit PATCH] Check for cmddir earlier

[martin f. krafft]

Without cmddir, tg is basically useless, even do_help() needs it, so
check it first and die hard if not found

Signed-off-by: martin f. krafft <madduck@debian.org>
---
 tg.sh |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/tg.sh b/tg.sh
index ea22544..f3d1323 100644
--- a/tg.sh
+++ b/tg.sh
@@ -237,6 +237,9 @@ do_help()
 
 ## Startup
 
+[ -d "@cmddir@" ] ||
+	die "No command directory: '@cmddir@'"
+
 args_saved="$@"
 while [ -n "$1" ]; do
 	case "$1" in
@@ -263,9 +266,6 @@ tg="tg"
 setup_ours
 setup_hook "pre-commit"
 
-[ -d "@cmddir@" ] ||
-	die "No command directory: '@cmddir@'"
-
 ## Dispatch
 
 # We were sourced from another script for our utility functions;
-- 
1.6.0.2

[TopGit PATCH] Print help output when no command is given

[martin f. krafft]

As much as I love your message (although you really ought not be
throwing around ducks!), this is more user-friendly.

Signed-off-by: martin f. krafft <madduck@debian.org>
---
 tg.sh |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/tg.sh b/tg.sh
index f3d1323..e0d62e7 100644
--- a/tg.sh
+++ b/tg.sh
@@ -278,7 +278,7 @@ if [ "$1" = "-r" ]; then
 fi
 
 cmd="$1"
-[ -n "$cmd" ] || die "He took a duck in the face at two hundred and fifty knots"
+[ -n "$cmd" ] || { do_help; exit 1; }
 shift
 
 case "$cmd" in
-- 
1.6.0.2

Re: [TopGit PATCH] Check for help invocation before setup

[Petr Baudis]

On Wed, Nov 19, 2008 at 05:03:40PM +0100, martin f. krafft wrote:
> diff --git a/tg.sh b/tg.sh
> index 2961106..40c4ab7 100644
> --- a/tg.sh
> +++ b/tg.sh
> @@ -235,6 +235,20 @@ do_help()
>  	fi
>  }
>  
> +## Startup
> +
> +args_saved="$@"
> +while [ -n "$1" ]; do
> +	case "$1" in
> +	help|--help|-h)
> +		shift
> +		do_help "$1"
> +		exit 1;;
> +	esac
> +	shift
> +done
> +set -- $args_saved
> +unset args_saved
>  
>  ## Initial setup

Huh, why do you actually need $args_saved at all? :-) This is bound to
do horrible things with space-containing arguments etc., I think. You
don't need to do the outer shift and then drop $args_saved altogether,
no?

-- 
				Petr "Pasky" Baudis
People who take cold baths never have rheumatism, but they have
cold baths.

Re: [TopGit PATCH] Check for help invocation before setup

[martin f krafft]

[-- Attachment #1: Type: text/plain, Size: 1606 bytes --]

also sprach Petr Baudis <pasky@suse.cz> [2008.11.19.1924 +0100]:
> > +args_saved="$@"
> > +while [ -n "$1" ]; do
> > +	case "$1" in
> > +	help|--help|-h)
> > +		shift
> > +		do_help "$1"
> > +		exit 1;;
> > +	esac
> > +	shift
> > +done
> > +set -- $args_saved
> > +unset args_saved
> >  
> >  ## Initial setup
> 
> Huh, why do you actually need $args_saved at all? :-) This is bound to
> do horrible things with space-containing arguments etc., I think. You
> don't need to do the outer shift and then drop $args_saved altogether,
> no?

I figured I need to restore $@ for others to consume, e.g. when you
check for -r later.

The challenge is to identify help|--help|-h anywhere on the command
line. Thus, you need to iterate, or do some weird matching against
$*.

When you iterate in a for loop, it's not easy to get at the next
argument, except if you use a state machine. I wanted to avoid that.

But you are right, I am wreaking havoc with space-containing
arguments. I will have to go back and rework this.

I take it you agree with the general principle though?

-- 
 .''`.   martin f. krafft <madduck@debian.org>
: :'  :  proud Debian developer, author, administrator, and user
`. `'`   http://people.debian.org/~madduck - http://debiansystem.info
  `-  Debian - when you have better things to do than fixing systems
 
"it is easier to be a lover than a husband for the simple reason
 that it is more difficult to be witty every day
 than to say pretty things from time to time."
                                                   -- honoré de balzac

[-- Attachment #2: Digital signature (see http://martin-krafft.net/gpg/) --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [TopGit PATCH] Check for help invocation before setup

[Petr Baudis]

On Wed, Nov 19, 2008 at 07:32:34PM +0100, martin f krafft wrote:
> also sprach Petr Baudis <pasky@suse.cz> [2008.11.19.1924 +0100]:
> > > +args_saved="$@"
> > > +while [ -n "$1" ]; do
> > > +	case "$1" in
> > > +	help|--help|-h)
> > > +		shift
> > > +		do_help "$1"
> > > +		exit 1;;
> > > +	esac
> > > +	shift
> > > +done
> > > +set -- $args_saved
> > > +unset args_saved
> > >  
> > >  ## Initial setup
> > 
> > Huh, why do you actually need $args_saved at all? :-) This is bound to
> > do horrible things with space-containing arguments etc., I think. You
> > don't need to do the outer shift and then drop $args_saved altogether,
> > no?
> 
> I figured I need to restore $@ for others to consume, e.g. when you
> check for -r later.
> 
> The challenge is to identify help|--help|-h anywhere on the command
> line. Thus, you need to iterate, or do some weird matching against
> $*.
> 
> When you iterate in a for loop, it's not easy to get at the next
> argument, except if you use a state machine. I wanted to avoid that.

Oh, oops - I missed the loop, I should get some lunch. ;-)
Thus, something like this?

	check_help() { while [ -n "$1" ]; do ...; shift; done }
	check_help "$@"

-- 
				Petr "Pasky" Baudis
People who take cold baths never have rheumatism, but they have
cold baths.

Re: [TopGit PATCH] Check for help invocation before setup

[martin f krafft]

[-- Attachment #1: Type: text/plain, Size: 777 bytes --]

also sprach Petr Baudis <pasky@suse.cz> [2008.11.19.1939 +0100]:
> Oh, oops - I missed the loop, I should get some lunch. ;-)
> Thus, something like this?
> 
> 	check_help() { while [ -n "$1" ]; do ...; shift; done }
> 	check_help "$@"

Why didn't I think of that???

I just sent an updated patch without threading, sorry. tg-mail just
fires without confirmation... :/

-- 
 .''`.   martin f. krafft <madduck@debian.org>
: :'  :  proud Debian developer, author, administrator, and user
`. `'`   http://people.debian.org/~madduck - http://debiansystem.info
  `-  Debian - when you have better things to do than fixing systems
 
"the pure and simple truth is rarely pure and never simple."
                                                      -- oscar wilde

[-- Attachment #2: Digital signature (see http://martin-krafft.net/gpg/) --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [TopGit PATCH] Check for help invocation before setup

[Bert Wesarg]

Hi,

On Wed, Nov 19, 2008 at 17:03, martin f. krafft <madduck@debian.org> wrote:
> The user ought to be able to call `tg help` from anywhere in the
> filesystem, not just Git repositories, so the help parsing has to happen
> before the calls to git git binary.
>
> Debian bug: #501982
I really second this. Plus, I think its crucial to check that we are
not inside the .git directory before setting up topgit. Because git
rev-parse --git-dir only works in the top .git dir, not deeper (i.e.
inside .git/refs) and will always return ".".

	$(git rev-parse --is-inside-work-tree) ||
		die "Not in a git working directory"

Bert

TopGit: ensure worktree (was: [TopGit PATCH] Check for help invocation before setup)

[martin f krafft]

[-- Attachment #1: Type: text/plain, Size: 1153 bytes --]

I think you wanted to CC Petr, not Junio. It's also useful to CC
Debian bugs if you know of them. :)

also sprach Bert Wesarg <bert.wesarg@googlemail.com> [2008.11.21.1019 +0100]:
> I really second this. Plus, I think its crucial to check that we are
> not inside the .git directory before setting up topgit. Because git
> rev-parse --git-dir only works in the top .git dir, not deeper (i.e.
> inside .git/refs) and will always return ".".
> 
> 	$(git rev-parse --is-inside-work-tree) ||
> 		die "Not in a git working directory"

Thanks, noted. I am still working on this patch, since
evaluating $argv/checking for --help at this early stage basically
requires a rewrite of the CLI parsing, ideally using POSIX getopt or
something similar, which takes much of the load away.

Anyway, Patch for your proposal forthcoming.

-- 
 .''`.   martin f. krafft <madduck@debian.org>
: :'  :  proud Debian developer, author, administrator, and user
`. `'`   http://people.debian.org/~madduck - http://debiansystem.info
  `-  Debian - when you have better things to do than fixing systems
 
(a)bort, (r)etry, (p)retend this never happened

[-- Attachment #2: Digital signature (see http://martin-krafft.net/gpg/) --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

[TopGit PATCH] Ensure we are inside a Git worktree

[martin f. krafft]

Bert Wesarg suggests to check that we are not inside the .git directory before
setting up topgit. Because git rev-parse --git-dir only works in the top .git
dir, not deeper (i.e. inside .git/refs) and will always return "."

This patch thus causes topgit to die early on when it's called from outside of
a Git repository, or from underneath .git and outputs error messages
accordingly.

This temporarily makes it even more impossible to call tg --help, but I expect
to have that fixed soon too.

Signed-off-by: martin f. krafft <madduck@debian.org>

---
 tg.sh |   16 ++++++++++++++++
 1 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/tg.sh b/tg.sh
index 8c23d26..f8c8de4 100644
--- a/tg.sh
+++ b/tg.sh
@@ -17,6 +17,20 @@ die()
 	exit 1
 }
 
+# Make sure we are in the worktree, not under .git; die otherwise
+ensure_git_repo_or_die()
+{
+	local is_inside_repo is_inside_git_dir
+	is_inside_repo=1
+	is_inside_git_dir=$(git rev-parse --is-inside-git-dir 2>/dev/null) ||
+		is_inside_repo=0
+
+	case "$is_inside_repo/$is_inside_git_dir" in
+	0*) die "Cannot run outside of a Git repository.";;
+	1/true) die "Cannot run from inside \`.git\` hierarchy, please switch to work-tree.";;
+	esac
+}
+
 # setup_hook NAME
 setup_hook()
 {
@@ -249,6 +263,8 @@ do_help()
 [ -d "@cmddir@" ] ||
 	die "No command directory: '@cmddir@'"
 
+ensure_git_repo_or_die
+
 ## Initial setup
 
 set -e
-- 
tg: (2ea19b6..) fixes/ensure-worktree (depends on: upstream)

Re: TopGit: ensure worktree (was: [TopGit PATCH] Check for help invocation before setup)

[Bert Wesarg]

On Fri, Nov 21, 2008 at 13:06, martin f krafft <madduck@debian.org> wrote:
> I think you wanted to CC Petr, not Junio. It's also useful to CC
> Debian bugs if you know of them. :)
I CC'ed Junio, because I think this git rev-parse --git-dir is a bug.
For example you can try starting git gui inside a .git dir.

>
> also sprach Bert Wesarg <bert.wesarg@googlemail.com> [2008.11.21.1019 +0100]:
>> I really second this. Plus, I think its crucial to check that we are
>> not inside the .git directory before setting up topgit. Because git
>> rev-parse --git-dir only works in the top .git dir, not deeper (i.e.
>> inside .git/refs) and will always return ".".
>>
>>       $(git rev-parse --is-inside-work-tree) ||
>>               die "Not in a git working directory"
>
> Thanks, noted. I am still working on this patch, since
> evaluating $argv/checking for --help at this early stage basically
> requires a rewrite of the CLI parsing, ideally using POSIX getopt or
> something similar, which takes much of the load away.
>
> Anyway, Patch for your proposal forthcoming.
A patch would depent on your --help patch, Because help should work everywhere.
So either I wait for you or do do it.

Bert
>
> --
>  .''`.   martin f. krafft <madduck@debian.org>
> : :'  :  proud Debian developer, author, administrator, and user
> `. `'`   http://people.debian.org/~madduck - http://debiansystem.info
>  `-  Debian - when you have better things to do than fixing systems
>
> (a)bort, (r)etry, (p)retend this never happened
>
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.9 (GNU/Linux)
>
> iEYEARECAAYFAkkmpDAACgkQIgvIgzMMSnUSKgCghXE+J9KegZ03tetnM3TIcyeM
> zn0AoJmYVzh/Kjxk7NFEpQw7USR0daXy
> =xl31
> -----END PGP SIGNATURE-----
>
>

Re: TopGit: ensure worktree (was: [TopGit PATCH] Check for help invocation before setup)

[Bert Wesarg]

On Fri, Nov 21, 2008 at 13:15, Bert Wesarg <bert.wesarg@googlemail.com> wrote:
> On Fri, Nov 21, 2008 at 13:06, martin f krafft <madduck@debian.org> wrote:
>> I think you wanted to CC Petr, not Junio. It's also useful to CC
>> Debian bugs if you know of them. :)
> I CC'ed Junio, because I think this git rev-parse --git-dir is a bug.
> For example you can try starting git gui inside a .git dir.
Oh, I send a patch only to petr, but forgot to cc the list ;-)

But your patch should be fine, thanks.

Bert

Re: TopGit: ensure worktree (was: [TopGit PATCH] Check for help invocation before setup)

[martin f krafft]

[-- Attachment #1: Type: text/plain, Size: 1504 bytes --]

also sprach Bert Wesarg <bert.wesarg@googlemail.com> [2008.11.21.1315 +0100]:
> I CC'ed Junio, because I think this git rev-parse --git-dir is
> a bug. For example you can try starting git gui inside a .git dir.

You might want to let him know of your intention. :)

> A patch would depent on your --help patch, Because help should
> work everywhere. So either I wait for you or do do it.

No, not depend, but the patch makes the --help situation slightly
worse, for now you cannot call --help inside .git anymore either.
This will force us to fix the --help situation quicker.

I've run out of time, for a while at least. If you want to pick up
the pieces and convert argument parsing to POSIX getopt, or maybe
even better, git-rev-parse --parseopt, then please go for it. You
can find my current TopGit branch at

  http://git.debian.org/?p=collab-maint/topgit.git;a=shortlog;h=refs/heads/fixes/independent-help
  git://git.debian.org/git/collab-maint/topgit.git, branch fixes/independent-help

If you use Debian: debcheckout topgit (requires devscripts 2.10.40)

Thanks,

-- 
 .''`.   martin f. krafft <madduck@debian.org>
: :'  :  proud Debian developer, author, administrator, and user
`. `'`   http://people.debian.org/~madduck - http://debiansystem.info
  `-  Debian - when you have better things to do than fixing systems
 
"heuristic is computer science jargon for 'doesn't actually work.'"
                                                     -- charlie reiman

[-- Attachment #2: Digital signature (see http://martin-krafft.net/gpg/) --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: TopGit: ensure worktree (was: [TopGit PATCH] Check for help invocation before setup)

[martin f krafft]

[-- Attachment #1: Type: text/plain, Size: 622 bytes --]

also sprach Bert Wesarg <bert.wesarg@googlemail.com> [2008.11.21.1318 +0100]:
> Oh, I send a patch only to petr, but forgot to cc the list ;-)

Maybe you can try to remember to CC me on topgit stuff in the future
too, since I am helping Petr out a bit as his release apprentice. :)

-- 
 .''`.   martin f. krafft <madduck@debian.org>
: :'  :  proud Debian developer, author, administrator, and user
`. `'`   http://people.debian.org/~madduck - http://debiansystem.info
  `-  Debian - when you have better things to do than fixing systems
 
i stopped fighting my inner demons.. we're all on the same side now.

[-- Attachment #2: Digital signature (see http://martin-krafft.net/gpg/) --]
[-- Type: application/pgp-signature, Size: 197 bytes --]