Re: [PATCH 22/83] builtin/apply: move 'unsafe_paths' global into 'struct apply_state'

Christian Couder [off-list ref] writes:

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.

On Tue, Apr 26, 2016 at 10:27 PM, Junio C Hamano [off-list ref] wrote:

Christian Couder [off-list ref] writes:

quoted

Signed-off-by: Christian Couder <redacted>
---
 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.