Re: [PATCH/RFC 4/4] Add interactive mode to git-shell for user-friendliness
- From
Johannes Sixt <j.sixt@viscovery.net>
- Date
- Jul 14, 2010, 10:27 UTC
- Message-ID
- <4C3D910B.7080401@viscovery.net>
- In-Reply-To
- <1279076475-27730-5-git-send-email-gdb@mit.edu>
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"
...
Show 19 quoted lines
> +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()?
Show 30 quoted lines
> @@ -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