Re: [PATCH v2 1/2] status: refactor output format to represent "default"

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Oct 18, 2012 at 08:57:59AM +0700, Nguyen Thai Ngoc Duy wrote:

> From: Jeff King <peff@xxxxxxxx>
> 
> 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". 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 <peff@xxxxxxxx>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx>
> ---
>  Two die()s with --long are moved to the next patch where --long is
>  introduced.

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.

Other than that, the patches look OK to me.

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


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]