On Tue, May 8, 2012 at 4:23 AM, Jeff King <peff@xxxxxxxx> wrote: > 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@xxxxxxxx> > --- > 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 -- 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