From: Christian Couder Date: Wed, 27 Apr 2016 12:14:40 GMT Subject: Re: [PATCH 22/83] builtin/apply: move 'unsafe_paths' global into 'struct apply_state' Message-ID: In-Reply-To: On Tue, Apr 26, 2016 at 10:27 PM, Junio C Hamano wrote: > Christian Couder writes: > >> Signed-off-by: Christian Couder >> --- >> builtin/apply.c | 9 +++++---- >> 1 file changed, 5 insertions(+), 4 deletions(-) >> >> diff --git a/builtin/apply.c b/builtin/apply.c >> index 506357c..c45e481 100644 >> --- a/builtin/apply.c >> +++ b/builtin/apply.c >> @@ -57,6 +57,8 @@ struct apply_state { >> int unidiff_zero; >> >> int update_index; >> + >> + int unsafe_paths; >> }; > > Having said > > I like the way this series moves only a few variables at a time to > limit the scope of each step. > > it gets irritating to see all these unnecessary blank lines in the > structure definition added by each step, which would mean all of > these patches need to fix them in the next reroll. The reason I added some blank lines is that I moved comments that were all in one block at the top. I moved each comment near the related variables and sometimes a comment is related to 2 variables, like in the extract below the comment that starts with "For "diff-stat" like behaviour,...": ------------------------------------------------------ /* --index updates the cache as well. */ int check_index; int unidiff_zero; int update_index; int unsafe_paths; int line_termination; /* * For "diff-stat" like behaviour, we keep track of the biggest change * we've seen, and the longest filename. That allows us to do simple * scaling. */ int max_change; int max_len; /* * Various "current state", notably line numbers and what * file (and how) we're patching right now.. The "is_xxxx" * things are flags, where -1 means "don't know yet". */ int linenr; ------------------------------------------------------ If I remove all the blank lines, I think it will make it harder to understand which comment belong to which variable(s). Maybe a compromise would be to just remove blank lines between the variables that don't have any related comment like "unidiff_zero", "update_index", "unsafe_paths" and "line_termination" above.