Re: [PATCH v16 06/10] imap-send: enable specifying the folder using the command line

[Junio C Hamano <gitster@pobox.com>]

Aditya Garg <gargaditya08@live.com> writes:

> Some users may very often want to imap-send messages to a folder
> other than the default set in the config. Add a command line
> argument for the same.
>
> Signed-off-by: Aditya Garg <gargaditya08@live.com>
> ---
>  Documentation/config/imap.adoc   |  6 ++++--
>  Documentation/git-imap-send.adoc | 15 +++++++++++----
>  imap-send.c                      |  9 ++++++++-
>  3 files changed, 23 insertions(+), 7 deletions(-)
>
> diff --git a/Documentation/config/imap.adoc b/Documentation/config/imap.adoc
> index 7c8b2dcce4..4682a6bd03 100644
> --- a/Documentation/config/imap.adoc
> +++ b/Documentation/config/imap.adoc
> @@ -1,7 +1,9 @@
>  imap.folder::
>  	The folder to drop the mails into, which is typically the Drafts
> -	folder. For example: "INBOX.Drafts", "INBOX/Drafts" or
> -	"[Gmail]/Drafts". Required.
> +	folder. For example: `INBOX.Drafts`, `INBOX/Drafts` or
> +	`[Gmail]/Drafts`. The IMAP folder to interact with MUST be specified;
> +	the value of this configuration variable is used as the fallback
> +	default value when the `--folder` option is not given.
>  
>  imap.tunnel::
>  	Command used to set up a tunnel to the IMAP server through which
> diff --git a/Documentation/git-imap-send.adoc b/Documentation/git-imap-send.adoc
> index 8adf0e5aac..4a0487b66e 100644
> --- a/Documentation/git-imap-send.adoc
> +++ b/Documentation/git-imap-send.adoc
> @@ -9,21 +9,23 @@ git-imap-send - Send a collection of patches from stdin to an IMAP folder
>  SYNOPSIS
>  --------
>  [verse]
> -'git imap-send' [-v] [-q] [--[no-]curl]
> +'git imap-send' [-v] [-q] [--[no-]curl] [(--folder|-f) <folder>]

This matches the _usage[] string.  Excellent.

>  DESCRIPTION
>  -----------
> -This command uploads a mailbox generated with 'git format-patch'
> +This command uploads a mailbox generated with `git format-patch`
>  into an IMAP drafts folder.  This allows patches to be sent as
>  other email is when using mail clients that cannot read mailbox
>  files directly. The command also works with any general mailbox
> -in which emails have the fields "From", "Date", and "Subject" in
> +in which emails have the fields `From`, `Date`, and `Subject` in
>  that order.
>  
>  Typical usage is something like:
>  
> -git format-patch --signoff --stdout --attach origin | git imap-send
> +------
> +$ git format-patch --signoff --stdout --attach origin | git imap-send
> +------

The above is small enough that it is OK to make the change
while-at-it, but it deserves a brief mention in the proposed log
message (e.g. "While at it, fix minor mark-up inconsistencies in the
existing documentation text").

> @@ -37,6 +39,11 @@ OPTIONS
>  --quiet::
>  	Be quiet.
>  
> +-f <folder>::
> +--folder=<folder>::
> +	Specify the folder in which the emails have to saved.
> +	For example: `--folder=[Gmail]/Drafts` or `-f INBOX/Drafts`.

Good.

> diff --git a/imap-send.c b/imap-send.c
> index c6e47ddc42..a4cccb9110 100644
> --- a/imap-send.c
> +++ b/imap-send.c
> @@ -46,12 +46,14 @@
>  
>  static int verbosity;
>  static int use_curl = USE_CURL_DEFAULT;
> +static char *opt_folder = NULL;

Let's lose "= NULL" here.

Do not explicitly initialize globals to 0 or NULL; let BSS take care
of the zero initialization, instead.

> -static const char * const imap_send_usage[] = { "git imap-send [-v] [-q] [--[no-]curl] < <mbox>", NULL };
> +static const char * const imap_send_usage[] = { "git imap-send [-v] [-q] [--[no-]curl] [(--folder|-f) <folder>] < <mbox>", NULL };
>  
>  static struct option imap_send_options[] = {
>  	OPT__VERBOSITY(&verbosity),
>  	OPT_BOOL(0, "curl", &use_curl, "use libcurl to communicate with the IMAP server"),
> +	OPT_STRING('f', "folder", &opt_folder, "folder", "specify the IMAP folder"),
>  	OPT_END()
>  };
>  
> @@ -1729,6 +1731,11 @@ int cmd_main(int argc, const char **argv)
>  
>  	argc = parse_options(argc, (const char **)argv, "", imap_send_options, imap_send_usage, 0);
>  
> +	if (opt_folder) {
> +		free(server.folder);
> +		server.folder = xstrdup(opt_folder);
> +	}

Good.  This matches the same care taken on the configuration side
that avoids leaking the value previously given.