Thread (4 messages) flat view 4 messages, 2 authors, 2017-06-29

Re: [PATCH 6/6] diff.c: detect blocks despite whitespace changes

From: Junio C Hamano <hidden>
Date: 2017-06-28 05:06:49

Junio C Hamano [off-list ref] writes:
Looking at the implementation of get_ws_cleaned_string() that is the
workhorse of emitted_symbol_cmp_no_ws(), it seems to be doing wrong
things for various "ignore whitespace" options (i.e. there is only
one implementation, while "git diff" family takes things like
"ignore space change", "ignore all whitespace", etc.), though.
This probably deserves a bit more illustration of how I envision the
code should evolve.

In the longer term, I would prefer to see emitted_symbol_cmp_no_ws()
to go and instead emitted_symbol_cmp() to take the diff options so
that it can change the behaviour of the comparison function based on
the -w/-b/--ignore-space-at-eol/etc. settings.  And compare two strings
in place.

For that, you may need a helper function that takes a pointer to a
character pointer, picks the next byte that matters while advancing
the pointer, and returns that byte.  The emitted_symbol_cmp(a, b)
which is not used for real comparison (i.e. ordering to see if a
sorts earlier than b) but for equivalence (i.e. considering various
whitespace-ignoring settings, does a and b matfch?) may become
something like:

        int
        emitted_symbol_eqv(struct emitted_diff_symbol *a,
                           struct emitted_diff_symbol *b,
                           const void *keydata) {
                struct diff_options *diffopt = keydata;
                const char *ap = a->line;
                const char *bp = b->line;

                while (1) {
                        int ca, cb;
                        ca = next_byte(&ap, diffopt);
                        cb = next_byte(&bp, diffopt);
                        if (ca != cb)
                                return 1; /* differs */
                        if (!ca)
                                return 0;
                };
        }                           

where next_byte() may look like:

        static int
        next_byte(const char **cp, struct diff_options *diffopt)
        {                  
                int retval;

        again:
                retval = **cp;
                if (!retval)
                        return retval; /* end of string */
                if (!isspace(retval)) {
                        (*cp)++; /* advance */
                        return retval;
                }

                switch (ignore_space_type(diffopt)) {
                case NOT_IGNORING:
                        (*cp)++; /* advance */
                        return retval;
                case IGNORE_SPACE_CHANGE:
                        while (**cp && isspace(**cp))
                                (*cp)++; /* squash consecutive spaces */
                        return ' '; /* normalize spaces with a SP */
                case IGNORE_ALL_SPACE:
                        (*cp)++; /* advance */
                        goto again;
                ... other cases here ...
                }
        }

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help