Re: [PATCH/RFC 4/4] Add interactive mode to git-shell for user-friendliness

[Johannes Sixt <j.sixt@viscovery.net>]

I don't have an immediate need for features implemented by this series,
but I think they can be useful occasionally.

Am 7/14/2010 5:01, schrieb Greg Brockman:
> --- a/shell.c
> +++ b/shell.c
> @@ -1,8 +1,11 @@
> +#include <stdio.h>

Is it really needed? Doesn't cache.h pull it in already?

> +
>  #include "cache.h"
...
> +static int run(const char *prog)
> +{
> +	pid_t pid, res;
> +	int w;
> +	pid = fork();
> +	if (pid == -1) {
> +		perror("fork");
> +		exit(-1);
> +	} else if ( pid == 0 ) {
> +		execl(prog, prog, (char *) NULL);
> +		if (prog[0] != '\0')
> +			fprintf(stderr, "unrecognized command '%s'\n", prog);
> +		exit(127);
> +	} else {
> +		do {
> +			res = waitpid (pid, &w, 0);
> +		} while (res == -1 && errno == EINTR);
> +	}
> +}

Is there a reason that you duplicate functionality offered by run_command()?

> @@ -81,8 +105,30 @@ int main(int argc, char **argv)
>  	 * We do not accept anything but "-c" followed by "cmd arg",
>  	 * where "cmd" is a very limited subset of git commands.
>  	 */
> -	else if (argc != 3 || strcmp(argv[1], "-c"))
> -		die("What do you think I am? A shell?");
> +	else if (argc != 3 || strcmp(argv[1], "-c")) {
> +		if (chdir(COMMAND_DIR))
> +			die("Sorry, the interactive git-shell is not enabled");
> +		for (;;) {
> +			printf("git> ");
> +			if (fgets(line, MAX_LINE_LEN, stdin) == NULL) {
> +				printf("\n");
> +				exit(0);
> +			}
> +
> +			if (line[strlen(line) - 1] == '\n')
> +				line[strlen(line) - 1] = '\0';
> +
> +			if (!strcmp(line, "quit") || !strcmp(line, "logout") ||
> +				   !strcmp(line, "exit")) {
> +				exit(0);
> +			} else if (!strcmp(line, "")) {
> +			} else if (is_valid_cmd_name(line)) {
> +				run(line);
> +			} else {
> +				fprintf(stderr, "invalid command format '%s'\n", line);
> +			}
> +		};
> +	}

I can imagine that this loop grows in the future, so I suggest to move it
to a separate function right from the beginning.

I think it would make sense to print a help message before the first prompt.

-- Hannes