Subject: Re: [PATCH v13 15/16] ls-tree: remove FIELD_*, just use MODE_*

From: Ævar Arnfjörð Bjarmason <[email-masked]>
Date: Mon, 21 Mar 2022 10:15:35 +0100

On Mon, Mar 21 2022, Teng Long wrote:

> From: Ævar Arnfjörð Bjarmason <[email-masked]>
>
> When we're picking where we should go in the optimized "show_tree"
> path there's no reason for why we need to convert our "cmdmode" of
> e.g. MODE_LONG into a FIELD_LONG_DEFAULT. Instead we can simply do
> those checks in the show_tree() function itself.
>
> Let's also make this code more future-proof by unrolling the hardcoded
> strmp() if/else if chain into something that checks a new "static
> struct" providing a bidirectional mapping between optimized formats
> and the ls_tree_cmdmode.
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <[email-masked]>
> Signed-off-by: Teng Long <[email-masked]>

For this & any other changes I suggested which amend code introduced
earlier in the series: Let's just fix that up into the respective
commits, if you agree this is a good direction that is.

I.e. if it's good to do Y let's not first do X and then change it to Y,
we can just start with Y.

Maybe that means none of these fix-up commitsa are left at the end, and
that's OK.

> ---
>  builtin/ls-tree.c | 98 ++++++++++++++++++++++-------------------------
>  1 file changed, 46 insertions(+), 52 deletions(-)
>
> diff --git a/builtin/ls-tree.c b/builtin/ls-tree.c
> index a271941540..3e756b5eee 100644
> --- a/builtin/ls-tree.c
> +++ b/builtin/ls-tree.c
> @@ -23,25 +23,13 @@ static int ls_options;
>  static struct pathspec pathspec;
>  static int chomp_prefix;
>  static const char *ls_tree_prefix;
> -#define FIELD_PATH_NAME 1
> -#define FIELD_SIZE (1 << 1)
> -#define FIELD_OBJECT_NAME (1 << 2)
> -#define FIELD_TYPE (1 << 3)
> -#define FIELD_MODE (1 << 4)
> -#define FIELD_DEFAULT 29 /* 11101 size is not shown to output by default */
> -#define FIELD_LONG_DEFAULT  (FIELD_DEFAULT | FIELD_SIZE)

I.e. this whole thing.

>  static const char *format;
> -static const char *default_format = "%(objectmode) %(objecttype) %(objectname)%x09%(path)";
> -static const char *long_format = "%(objectmode) %(objecttype) %(objectname) %(objectsize:padded)%x09%(path)";
> -static const char *name_only_format = "%(path)";
> -static const char *object_only_format = "%(objectname)";

And turning this into a loop can go with the initial --format change.

> [...]

All the changes below seemed related...