Re: [PATCH v3 07/10] builtin/replace: teach listing using short, medium or full formats

From: Christian Couder <chriscool@tuxfamily.org>
Date: Dec 28, 2013, 10:27 UTC
Message-ID: <20131228.112701.1954502091799602077.chriscool@tuxfamily.org>
In-Reply-To: <xmqqppojctsh.fsf@gitster.dls.corp.google.com>

From: Junio C Hamano <gitster@pobox.com>

Show 17 quoted lines

>
> Christian Couder <christian.couder@gmail.com> writes: 
>>
>> Ok, so would you prefer the following:
>>
>> - NAME_ONLY_REPLACE_FMT and "--format=name_only" instead of
>> SHORT_REPLACE_FMT and "--format=short"
>>
>> - NAME_AND_VALUE_REPLACE_FMT and "--format=name_and_value" instead of
>> MEDIUM_REPLACE_FMT and "--format=medium"
>>
>> - DEBUG_REPLACE_FMT and "--format=debug" instead of FULL _REPLACE_FMT
>> and "--format=full"
> 
> The end-user facing names are probably fine with short, medium,
> full, as long as what they show are clearly explained in the
> end-user documentation (patch 10/10 covers this).

Ok, I will try to improve on that.

> I have a hunch that we may later regret "full" when somebody wants
> to add even fuller information, though. It might be better spelled
> "long" instead;

Ok, I will use "long" instead.

> I'd rather see REPLACE_FMT_ as a prefix, not suffix.  Do we use
> common suffix for enum values elsewhere?

I don't see common suffix, but we have the following enums about formats:

* in builtin/commit.c:

static enum status_format {
        STATUS_FORMAT_NONE = 0,
        STATUS_FORMAT_LONG,
        STATUS_FORMAT_SHORT,
        STATUS_FORMAT_PORCELAIN,

        STATUS_FORMAT_UNSPECIFIED
} status_format = STATUS_FORMAT_UNSPECIFIED;

* in builtin/help.c:

enum help_format {
        HELP_FORMAT_NONE,
        HELP_FORMAT_MAN,
        HELP_FORMAT_INFO,
        HELP_FORMAT_WEB
};

* in commit.h

enum cmit_fmt {
        CMIT_FMT_RAW,
        CMIT_FMT_MEDIUM,
        CMIT_FMT_DEFAULT = CMIT_FMT_MEDIUM,
        CMIT_FMT_SHORT,
        CMIT_FMT_FULL,
        CMIT_FMT_FULLER,
        CMIT_FMT_ONELINE,
        CMIT_FMT_EMAIL,
        CMIT_FMT_USERFORMAT,

        CMIT_FMT_UNSPECIFIED
};

To conform to the above and what you suggest, I will send a new series using the following:

enum replace_format {
      REPLACE_FORMAT_SHORT,
      REPLACE_FORMAT_MEDIUM,
      REPLACE_FORMAT_LONG
};

Thanks, Christian.