Thanks a lot everyone for fixing this so quickly!
Please let me know if there is anything else I can do.
Best,
johannes
Johannes Postler
DevOps & Consulting
Txture - The Cloud Transformation Platform
Grabenweg 68
A-6020 Innsbruck
johannes.postler@xxxxxxxxx
www.txture.io
Johannes Postler
DevOps & Consulting
Txture - The Cloud Transformation Platform
Grabenweg 68
A-6020 Innsbruck
johannes.postler@xxxxxxxxx
www.txture.io
Follow us:
On Wed, Nov 4, 2020 at 8:26 PM Jeff King <peff@xxxxxxxx> wrote:
>
> On Wed, Nov 04, 2020 at 08:24:28AM -0500, Jeff King wrote:
>
> > The issue is that "--output" was never supposed to work with
> > format-patch. But a subtle change in the option parsing a while back
> > caused it to be respected. And as you noticed, the documentation
> > mistakenly mentions the option, since format-patch includes the standard
> > diff-options text.
> >
> > So one obvious fix would be to forbid it and adjust the documentation.
> > But because of the way the option parsers interact, it's surprisingly
> > hard to do so cleanly. It's actually easier to just make it do something
> > useful (i.e., behave like --stdout but sent to a file). So I did that
> > instead.
> >
> > [1/3]: format-patch: refactor output selection
> > [2/3]: format-patch: tie file-opening logic to output_directory
> > [3/3]: format-patch: support --output option
>
> Here's a re-roll taking into account the comments from Eric. The only
> thing I didn't do is rewrite --output as a format-patch option. As I
> said in the thread, I'd rather keep parity with how git-log works here
> (though I don't mind if somebody wants to do further clean up on top).
>
> [1/3]: format-patch: refactor output selection
> [2/3]: format-patch: tie file-opening logic to output_directory
> [3/3]: format-patch: support --output option
>
> builtin/log.c | 37 ++++++++++++++++++++++---------------
> t/t4014-format-patch.sh | 33 +++++++++++++++++++++++++++++++++
> 2 files changed, 55 insertions(+), 15 deletions(-)
>
>
> Range diff from v1:
>
> 1: 49e8b54549 ! 1: 9206d6852b format-patch: refactor output selection
> @@ Commit message
> slightly easier to follow now, and also will keep things sane when we
> add another output mode in a future patch.
>
> + We'll add a few tests as well, covering the mutual exclusion and the
> + fact that we are not confused by a configured output directory.
> +
> Signed-off-by: Jeff King <peff@xxxxxxxx>
>
> ## builtin/log.c ##
> @@ builtin/log.c: int cmd_format_patch(int argc, const char **argv, const char *pre
> - if (!output_directory && !use_stdout)
> - output_directory = config_output_directory;
> + if (use_stdout + !!output_directory > 1)
> -+ die(_("specify only one of --stdout, --output, and --output-directory"));
> ++ die(_("--stdout and --output-directory are mutually exclusive"));
>
> - if (!use_stdout)
> - output_directory = set_outdir(prefix, output_directory);
> @@ builtin/log.c: int cmd_format_patch(int argc, const char **argv, const char *pre
> /*
> * We consider <outdir> as 'outside of gitdir', therefore avoid
> * applying adjust_shared_perm in s-c-l-d.
> +
> + ## t/t4014-format-patch.sh ##
> +@@ t/t4014-format-patch.sh: test_expect_success 'format-patch -o overrides format.outputDirectory' '
> + test_path_is_dir patchset
> + '
> +
> ++test_expect_success 'format-patch forbids multiple outputs' '
> ++ rm -fr outdir &&
> ++ test_must_fail \
> ++ git format-patch --stdout --output-directory=outdir
> ++'
> ++
> ++test_expect_success 'configured outdir does not conflict with output options' '
> ++ rm -fr outfile outdir &&
> ++ test_config format.outputDirectory outdir &&
> ++ git format-patch --stdout &&
> ++ test_path_is_missing outdir
> ++'
> ++
> + test_expect_success 'format-patch --base' '
> + git checkout patchid &&
> +
> 2: 884c06861d ! 2: 9dc30924b2 format-patch: tie file-opening logic to output_directory
> @@ Commit message
> format-patch: tie file-opening logic to output_directory
>
> In format-patch we're either outputting to stdout or to individual files
> - in an output directory (which maybe just "./"). Our logic for whether to
> - open a new file for each patch is checked with "!use_stdout", but it is
> - equally correct to check for a non-NULL output_directory.
> + in an output directory (which may be just "./"). Our logic for whether
> + to open a new file for each patch is checked with "!use_stdout", but it
> + is equally correct to check for a non-NULL output_directory.
>
> The distinction will matter when we add a new single-stream output in a
> future patch, when only one of the three methods will want individual
> 3: 8befceb150 ! 3: 2b0fab9b50 format-patch: support --output option
> @@ builtin/log.c: int cmd_format_patch(int argc, const char **argv, const char *pre
> load_display_notes(&rev.notes_opt);
>
> - if (use_stdout + !!output_directory > 1)
> +- die(_("--stdout and --output-directory are mutually exclusive"));
> + if (use_stdout + rev.diffopt.close_file + !!output_directory > 1)
> - die(_("specify only one of --stdout, --output, and --output-directory"));
> ++ die(_("--stdout, --output, and --output-directory are mutually exclusive"));
>
> if (use_stdout) {
> setup_pager();
> @@ builtin/log.c: int cmd_format_patch(int argc, const char **argv, const char *pre
>
> ## t/t4014-format-patch.sh ##
> @@ t/t4014-format-patch.sh: test_expect_success 'format-patch -o overrides format.outputDirectory' '
> - test_path_is_dir patchset
> - '
> -
> -+test_expect_success 'format-patch forbids multiple outputs' '
> -+ rm -fr outfile outdir &&
> + test_expect_success 'format-patch forbids multiple outputs' '
> + rm -fr outdir &&
> + test_must_fail \
> +- git format-patch --stdout --output-directory=outdir
> ++ git format-patch --stdout --output-directory=outdir &&
> + test_must_fail \
> + git format-patch --stdout --output=outfile &&
> + test_must_fail \
> -+ git format-patch --stdout --output-directory=outdir &&
> -+ test_must_fail \
> + git format-patch --output=outfile --output-directory=outdir
> -+'
> -+
> -+test_expect_success 'configured outdir does not conflict with output options' '
> -+ rm -fr outfile outdir &&
> -+ test_config format.outputDirectory outdir &&
> -+ git format-patch --stdout &&
> + '
> +
> + test_expect_success 'configured outdir does not conflict with output options' '
> + rm -fr outfile outdir &&
> + test_config format.outputDirectory outdir &&
> + git format-patch --stdout &&
> + test_path_is_missing outdir &&
> + git format-patch --output=outfile &&
> -+ test_path_is_missing outdir
> -+'
> -+
> + test_path_is_missing outdir
> + '
> +
> +test_expect_success 'format-patch --output' '
> + rm -fr outfile &&
> + git format-patch -3 --stdout HEAD >expect &&
