Re: [RFC PATCH] revert: Removed top level --stderr= option

[Jani Nikula <jani@nikula.org>]

On Fri, 31 May 2013, Tomi Ollila <tomi.ollila@iki.fi> wrote:
> While looked good on paper, its attempted use caused confusion, complexity,
> and potential for information leak when passed through wrapper scripts.
> For slimmer code and to lessen demand for maintenance/support the set of
> commits which added top level --stderr= option is now reverted.
> ---
>
> Change was easy, commit message hard. Opinions? Revert is easiest to do now.
> Also, if someone comes with a novel idea how to utilize --stderr option
> please tell it now -- I'd be most eager to hear it :D
>
>
> This change was done the following way:
>
> $ git checkout -b rvrt b9020448bd
> $ git reset --hard HEAD^^^^
> $ git reset b9020448bd
> $ git commit -a
> $ git diff HEAD~5..HEAD

Protip ;)

$ git revert -n b9020448bd^^^^..b9020448bd
$ git commit

BR,
Jani.

>
> (last one to reveal HEAD~5 & HEAD have identical trees).
>
> Question why:
> id:20130521195549.6550.53636@thinkbox.jade-hamburg.de
>
> Good reason why not:
> id:1369934016-22308-1-git-send-email-amdragon@mit.edu
>
>
>  NEWS               |  5 -----
>  man/man1/notmuch.1 |  7 -------
>  notmuch-client.h   |  1 -
>  notmuch.c          | 32 --------------------------------
>  test/help-test     |  9 ---------
>  5 files changed, 54 deletions(-)
>
> diff --git a/NEWS b/NEWS
> index 80abd97..a7f2ec6 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -35,11 +35,6 @@ Top level option to specify configuration file
>    It's now possible to specify the configuration file to use on the
>    command line using the `notmuch --config=FILE` option.
>  
> -Top level option to redirect writes to stderr
> -
> -  With `notmuch --stderr=FILE` all writes to stderr are redirected to
> -  the specified file. If FILE is '-', stderr is redirected to stdout.
> -
>  Deprecated commands "part" and "search-tags" are removed.
>  
>  Bash command-line completion
> diff --git a/man/man1/notmuch.1 b/man/man1/notmuch.1
> index f5ca0ad..033cc10 100644
> --- a/man/man1/notmuch.1
> +++ b/man/man1/notmuch.1
> @@ -76,14 +76,7 @@ Print the installed version of notmuch, and exit.
>  
>  Specify the configuration file to use. This overrides any
>  configuration file specified by ${NOTMUCH_CONFIG}.
> -.RE
> -
> -.RS 4
> -.TP 4
> -.B \-\-stderr=FILE
>  
> -Redirect all writes to stderr to the specified file.
> -If FILE is '-', stderr is redirected to stdout.
>  .RE
>  
>  .SH COMMANDS
> diff --git a/notmuch-client.h b/notmuch-client.h
> index 4a3c7ac..45749a6 100644
> --- a/notmuch-client.h
> +++ b/notmuch-client.h
> @@ -54,7 +54,6 @@ typedef GMimeCipherContext notmuch_crypto_context_t;
>  #include <sys/stat.h>
>  #include <sys/time.h>
>  #include <unistd.h>
> -#include <fcntl.h>
>  #include <dirent.h>
>  #include <errno.h>
>  #include <signal.h>
> diff --git a/notmuch.c b/notmuch.c
> index 15e90c8..f51a84f 100644
> --- a/notmuch.c
> +++ b/notmuch.c
> @@ -251,32 +251,6 @@ notmuch_command (notmuch_config_t *config,
>      return 0;
>  }
>  
> -static int
> -redirect_stderr (const char * stderr_file)
> -{
> -    if (strcmp (stderr_file, "-") == 0) {
> -	if (dup2 (STDOUT_FILENO, STDERR_FILENO) < 0) {
> -	    perror ("dup2");
> -	    return 1;
> -	}
> -    } else {
> -	int fd = open (stderr_file, O_WRONLY|O_CREAT|O_TRUNC, 0666);
> -	if (fd < 0) {
> -	    fprintf (stderr, "Error: Cannot redirect stderr to '%s': %s\n",
> -		     stderr_file, strerror (errno));
> -	    return 1;
> -	}
> -	if (fd != STDERR_FILENO) {
> -	    if (dup2 (fd, STDERR_FILENO) < 0) {
> -		perror ("dup2");
> -		return 1;
> -	    }
> -	    close (fd);
> -	}
> -    }
> -    return 0;
> -}
> -
>  int
>  main (int argc, char *argv[])
>  {
> @@ -285,7 +259,6 @@ main (int argc, char *argv[])
>      const char *command_name = NULL;
>      command_t *command;
>      char *config_file_name = NULL;
> -    char *stderr_file = NULL;
>      notmuch_config_t *config;
>      notmuch_bool_t print_help=FALSE, print_version=FALSE;
>      int opt_index;
> @@ -295,7 +268,6 @@ main (int argc, char *argv[])
>  	{ NOTMUCH_OPT_BOOLEAN, &print_help, "help", 'h', 0 },
>  	{ NOTMUCH_OPT_BOOLEAN, &print_version, "version", 'v', 0 },
>  	{ NOTMUCH_OPT_STRING, &config_file_name, "config", 'c', 0 },
> -	{ NOTMUCH_OPT_STRING, &stderr_file, "stderr", '\0', 0 },
>  	{ 0, 0, 0, 0, 0 }
>      };
>  
> @@ -315,10 +287,6 @@ main (int argc, char *argv[])
>  	return 1;
>      }
>  
> -    if (stderr_file && redirect_stderr (stderr_file) != 0) {
> -	/* error already printed */
> -	return 1;
> -    }
>      if (print_help)
>  	return notmuch_help_command (NULL, argc - 1, &argv[1]);
>  
> diff --git a/test/help-test b/test/help-test
> index bd0111c..f7df725 100755
> --- a/test/help-test
> +++ b/test/help-test
> @@ -9,13 +9,4 @@ test_expect_success 'notmuch help' 'notmuch help'
>  test_expect_success 'notmuch help tag' 'notmuch help tag'
>  test_expect_success 'notmuch --version' 'notmuch --version'
>  
> -test_begin_subtest "notmuch --stderr=stderr help %"
> -notmuch --stderr=stderr help %
> -test_expect_equal "$(cat stderr)" "
> -Sorry, % is not a known command. There's not much I can do to help."
> -
> -test_begin_subtest "notmuch --stderr=- help %"
> -test_expect_equal "$(notmuch --stderr=- help %)" "
> -Sorry, % is not a known command. There's not much I can do to help."
> -
>  test_done
> -- 
> 1.8.0
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch