On Mon, Mar 21 2022, Teng Long wrote: > From: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> > > 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 <avarab@xxxxxxxxx> > Signed-off-by: Teng Long <dyroneteng@xxxxxxxxx> 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...