Re: [Monotone-devel] Review of diff-p branch

[Nathaniel Smith]

On Tue, Jul 11, 2006 at 02:48:22PM -0700, Zack Weinberg wrote:
> On 7/11/06, Nathaniel Smith <address@hidden> wrote:
> >+/* Find, and write to ENCLOSER, the nearest line before POS which
> >matches
> >+ ENCLOSER_PATTERN.  We remember the last line scanned, and the
> >matched, to
> >+   avoid duplication of effort.  */
> ...
> >^^ I think I'd feel more comfortable here with some I()'s scattered
> >around here?  It is Clever, and involves Pointers, you see.
> 
> Not really pointers, just iterators and a lot of complicated
> vector-index arithmetic, but see attached; maybe it's clearer?

std::vector iterators are pretty thin wrappers around pointers -- have
a lot of the same risks.  The patch looks fine; I just feel more
warm-and-fuzzy when complicated arithmetic is going on, if the
assumptions are also documented in code :-).

Looks fine to commit.

> ...
> >The way the unidiff and context diff writers go back and parse
> >their own output seems a bit... well, odd, anyway?
> 
> The requirement is to pass find_encloser() an index which is one less
> than the first changed line (+, -, or !) in the hunk; this is not
> always a_begin+ctx, as a previous version tried.  I could have the
> writers keep track of which line this is, but honestly it seemed
> clearer to me to do it this way.

Fair enough.  It's hardly the weirdest bit of code in monotone :-).

-- Nathaniel

-- 
"...these, like all words, have single, decontextualized meanings: everyone
knows what each of these words means, everyone knows what constitutes an
instance of each of their referents.  Language is fixed.  Meaning is
certain.  Santa Claus comes down the chimney at midnight on December 24."
  -- The Language War, Robin Lakoff