Re: [PATCH 2/5] status: refactor colopts handling
On Tue, May 8, 2012 at 4:23 AM, Jeff King <peff@peff.net> wrote:
Show 26 quoted lines
> The current code reads the config and command-line options
> into a separate "colopts" variable, and then copies the
> contents of that variable into the "struct wt_status". We
> can eliminate the extra variable and copy just write
> straight into the wt_status struct.
>
> This simplifies the "status" code a little bit.
> Unfortunately, it makes the "commit" code one line more
> complex; a side effect of the separate variable was that
> "commit" did not copy the colopts variable, so any
> column.status configuration had no effect.
>
> The result still ends up cleaner, though. In the previous
> version, it was unclear whether commit simply forgot to copy
> the colopt variable, or whether it was intentional. Now it
> explicitly turns off column options. Furthermore, if commit
> later learns to respect column.status, this will make the
> end result simpler. I punted on just adding that feature
> now, because it was sufficiently non-obvious that it should
> not go into a refactoring patch.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> For the reasons mentioned above, this ended up as a less impressive
> cleanup than I had hoped. I think it's still worth doing, but I won't be
> too sad if it gets dropped.
I'm neutral. But this patch reduces one line so it wins a "yes" from me.
> builtin/commit.c | 13 ++++++-------
> wt-status.h | 2 +-
> 2 files changed, 7 insertions(+), 8 deletions(-)
--
Duy
Nguyen Thai Ngoc Duy <pclouds@gmail.com>