From: Martin Pieuchot <mpi@openbsd.org>
Subject: Re: tog(1) log by default++
To: gameoftrees@openbsd.org
Date: Fri, 28 Feb 2020 14:56:27 +0100

On 20/02/20(Thu) 17:24, Stefan Sperling wrote:
> On Fri, Feb 14, 2020 at 05:15:32PM +0100, Stefan Sperling wrote:
> > On Fri, Feb 14, 2020 at 04:59:32PM +0100, Stefan Sperling wrote:
> > > On Fri, Feb 14, 2020 at 04:39:48PM +0100, Martin Pieuchot wrote:
> > > > If the second argument on the command line doesn't match a command name
> > > > let the tool to assume it's a path.  In other words the following:
> > > > 
> > > > 	$ tog kern/kern_sync.c
> > > > 
> > > > Becomes an alias for:
> > > > 	
> > > > 	$ tog log kern/kern_sync.c
> > > > 
> > > > This is similar to what tig(1) does and it helps me being lazy :o)
> > > 
> > > This behaviour already existed once and was reverted in
> > > commit 3642c4c6513e3536dc77e8f2b7a2402d1aa916a7
> > > 
> > > I don't recall why I reverted it. I suspect it was to keep the
> > > 'command subcommand' syntax consistent across both got(1) and tog(1).
> > > And I didn't like the old implementation I had, but yours looks cleaner.
> > 
> > One problem is that 'tog foo' for non-existent foo no longer shows usage
> > information. It says "tog: no git repository found" or "tog: no such entry
> > found in tree" depending on the contents of the current working directory,
> > without any further hints.
> > 
> > Perhaps a custom error message could be added which says something like
> > "tog: 'foo' is neither a known command nor a path" when it fails, followed
> > by usage info?
> 
> Here is a diff that shows how I would implement this if we really want it.
> 
> A reason against removing make_argv() is that getopt inside the command's
> main function will otherwise process the whole original argv. This can
> lead to rather confusing usage errors when argv[0] is not a command name.
> So I've decided to keep it.
> 
> Does this diff work for you?
> (please ensure your tree is up-to-date before applying this)

It does.  I don't believe it's necessary to print the "alternative
usage" though.  Thanks.

> diff 6962eb72119c221eca83da6b7b3bd76f730d6898 /home/stsp/src/got
> blob - dc2613f1948e568e578c8d9c6b54e1296b91ca7d
> file + tog/tog.1
> --- tog/tog.1
> +++ tog/tog.1
> @@ -21,9 +21,13 @@
>  .Nd Git repository browser
>  .Sh SYNOPSIS
>  .Nm
> -.Ar command
> +.Op Ar command
>  .Op Fl h
>  .Op Ar arg ...
> +.Pp
> +.Nm
> +.Ar path
> +.El
>  .Sh DESCRIPTION
>  .Nm
>  is an interactive read-only browser for Git repositories.
> @@ -37,6 +41,8 @@ supports several types of views which display reposito
>  Displays commits in the repository's history.
>  This view is displayed initially if no
>  .Ar command
> +is specified, or if just a
> +.Ar path
>  is specified.
>  .It Diff view
>  Displays changes made in a particular commit.
> blob - 9e8009077b052e09ca9a672a6324e575c6116de3
> file + tog/tog.c
> --- tog/tog.c
> +++ tog/tog.c
> @@ -5308,10 +5308,12 @@ list_commands(void)
>  __dead static void
>  usage(int hflag)
>  {
> -	fprintf(stderr, "usage: %s [-h] [-V | --version] [command] [arg ...]\n",
> -	    getprogname());
> -	if (hflag)
> +	fprintf(stderr, "usage: %s [-h] [-V | --version] [command] "
> +	    "[arg ...]\n", getprogname());
> +	if (hflag) {
>  		list_commands();
> +		fprintf(stderr, "alternative usage: %s path\n", getprogname());
> +	}
>  	exit(1);
>  }
>  
> @@ -5347,6 +5349,7 @@ main(int argc, char *argv[])
>  	     { "version", no_argument, NULL, 'V' },
>  	     { NULL, 0, NULL, 0}
>  	};
> +	int is_implicit_log_cmd = 0;
>  
>  	setlocale(LC_CTYPE, "");
>  
> @@ -5384,7 +5387,7 @@ main(int argc, char *argv[])
>  	} else {
>  		int i;
>  
> -		/* Did the user specific a command? */
> +		/* Did the user specify a command? */
>  		for (i = 0; i < nitems(tog_commands); i++) {
>  			if (strncmp(tog_commands[i].name, argv[0],
>  			    strlen(argv[0])) == 0) {
> @@ -5392,23 +5395,40 @@ main(int argc, char *argv[])
>  				break;
>  			}
>  		}
> +	}
>  
> -		if (cmd == NULL) {
> -			fprintf(stderr, "%s: unknown command '%s'\n",
> -			    getprogname(), argv[0]);
> -			list_commands();
> -			return 1;
> -		}
> +	if (cmd == NULL) {
> +		if (argc != 1)
> +			usage(0);
> +		/* No command specified; try log with a path */
> +		cmd = &tog_commands[0];
> +		cmd_argv = make_argv(cmd->name, argv[0]);
> +		argc = 2;
> +		is_implicit_log_cmd = 1;
> +		error = cmd->cmd_main(argc, cmd_argv);
> +	} else {
> +		if (hflag)
> +			cmd->cmd_usage();
> +		else
> +			error = cmd->cmd_main(argc, cmd_argv ? cmd_argv : argv);
>  	}
>  
> -	if (hflag)
> -		cmd->cmd_usage();
> -	else
> -		error = cmd->cmd_main(argc, cmd_argv ? cmd_argv : argv);
> -
>  	endwin();
> -	free(cmd_argv);
> -	if (error && error->code != GOT_ERR_CANCELLED)
> -		fprintf(stderr, "%s: %s\n", getprogname(), error->msg);
> +	if (cmd_argv) {
> +		int i;
> +		for (i = 0; i < argc; i++)
> +			free(cmd_argv[i]);
> +		free(cmd_argv);
> +	}
> +
> +	if (error && error->code != GOT_ERR_CANCELLED) {
> +		if (is_implicit_log_cmd &&
> +		    error->code == GOT_ERR_NO_TREE_ENTRY) {
> +			fprintf(stderr, "%s: '%s' is no known command or path\n",
> +			    getprogname(), argv[0]);
> +			usage(1);
> +		} else
> +			fprintf(stderr, "%s: %s\n", getprogname(), error->msg);
> +	}
>  	return 0;
>  }
> 
>