Re: [PATCH v3] status: refactor output format to represent "default" and add --long

2012-10-18 Thread Jeff King
On Thu, Oct 18, 2012 at 02:16:11PM -0700, Junio C Hamano wrote:

> I guess combining both is fine, but then the commit is no longer "in
> preparation for adding" the option, but it already adds "--long", I
> would think.

Maybe a better commit message is:

-- >8 --
Subject: status: add --long output format option

You can currently set the output format to --short or
--porcelain. There is no --long, because we default to it
already. However, you may want to override an alias that
uses "--short" to get back to the default.

This requires a little bit of refactoring, because currently
we use STATUS_FORMAT_LONG internally to mean the same as
"the user did not specify anything". By expanding the enum
to include STATUS_FORMAT_NONE, we can distinguish between
the implicit and explicit cases. This effects these
conditions:

  1. The user has asked for NUL termination. With NONE, we
 currently default to turning on the porcelain mode.
 With an explicit --long, we would in theory use NUL
 termination with the long mode, but it does not support
 it. So we can just complain and die.

  2. When an output format is given to "git commit", we
 default to "--dry-run". This behavior would now kick in
 when "--long" is given, too.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3] status: refactor output format to represent "default" and add --long

2012-10-18 Thread Junio C Hamano
Nguyễn Thái Ngọc Duy   writes:

> From: Jeff King 
>
> When deciding which output format to use, we default an internal enum
> to STATUS_FORMAT_LONG and modify it if "--porcelain" or "--short" is
> given. If this enum is set to LONG, then we know the user has not
> specified any format, and we can kick in default behaviors. This works
> because there is no "--long" which they could use to explicitly
> specify LONG.
>
> Let's expand the enum to have an explicit STATUS_FORMAT_NONE, in
> preparation for adding "--long", which can be used to override --short
> or --porcelain. Then we can distinguish between LONG and NONE when
> setting other defaults. There are two such cases:
>
>   1. The user has asked for NUL termination. With NONE, we
>  currently default to turning on the porcelain mode.
>  With an explicit --long, we would in theory use NUL
>  termination with the long mode, but it does not support
>  it. So we can just complain and die.
>
>   2. When an output format is given to "git commit", we
>  default to "--dry-run". This behavior would now kick in
>  when "--long" is given, too.
>
> Signed-off-by: Jeff King 
> Signed-off-by: Nguyễn Thái Ngọc Duy 
> ---
>  On Thu, Oct 18, 2012 at 9:03 AM, Jeff King  wrote:
>  > I think that is fine to split it like this, but you would want to update
>  > the commit message above. Probably just remove those two cases and say
>  > something like:
>  >
>  >   Note that you cannot actually trigger STATUS_FORMAT_LONG, as we do
>  >   not yet have "--long"; that will come in a follow-on patch.
>  >
>  > And then move the reasoning for how "--long" works with each case into
>  > the commit message of the next patch.
>
>  Nope, it's hard to split the explanation in two (at least to me),
>  which may mean that the split does not make sense.

I guess combining both is fine, but then the commit is no longer "in
preparation for adding" the option, but it already adds "--long", I
would think.

Will queue.

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3] status: refactor output format to represent "default" and add --long

2012-10-18 Thread Jeff King
On Thu, Oct 18, 2012 at 09:15:50PM +0700, Nguyen Thai Ngoc Duy wrote:

>  On Thu, Oct 18, 2012 at 9:03 AM, Jeff King  wrote:
>  > I think that is fine to split it like this, but you would want to update
>  > the commit message above. Probably just remove those two cases and say
>  > something like:
>  >
>  >   Note that you cannot actually trigger STATUS_FORMAT_LONG, as we do
>  >   not yet have "--long"; that will come in a follow-on patch.
>  >
>  > And then move the reasoning for how "--long" works with each case into
>  > the commit message of the next patch.
> 
>  Nope, it's hard to split the explanation in two (at least to me),
>  which may mean that the split does not make sense.

Yeah, that was the same place that I had difficulty when writing the
original. This version looks OK to me.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html