Re: Fixing blame's eol sensitivity (plans for the last round)

On 12/7/05, Peter N. Lundblad <peter@famlundblad.se> wrote:
> On Wed, 7 Dec 2005, Erik Huelsmann wrote:
>
> > On 12/7/05, Julian Foad <julianfoad@btopenworld.com> wrote:
> > Erik Huelsmann wrote:
> > > So, to cut a long story short: I'll be creating svn_client_blame3,
> > > which takes the same blame callback, but possibly includes eols in the
> > > when returned line value. When a callback is passed to
> > > svn_client_blame2, it keeps its current behaviour of
> > > not-eating-CR's-but-eating-LFs.
> >
> > From an API point of view, that sounds too messy. Just pick a good interface
> > (either one that returns the EOLs, or one that doesn't) and stick with it. I
> > don't see the need for a run-time option.
>
> >Well, then we need an interface which marshalls the EOL style to the
> >blame callback, because in the XML output you *don't* want EOLs
> >included, whereas in the 'normal' case, you *do* want the correct EOLs
> >included....
>
> As far as I remember, we don't output the lines in XML at all because of
> encoding issues.

Exactly. And because of it, we want to be able to tell the difference
between the eol markers from the actual text on the blamed line.

> And if we did, why wouldn't we want the EOL markers in
> the XML output?
>
>
> >As I said, I can also rev the blame callback api and include the eol
> >marker in each call to the callback, but that too seems ugly...

> Why is that ugly? Isn't the eol marker part of the line?

I see I'm making not much sense. I'll give the current and desired
situations below. The desired situation is what I'm trying to
establish (ie. is what I *think* is desired).

Let's take line 500 from a file foo.txt which has line ending style
'native'. This means the line looks like this when seen by the blame
algorithm:

  "Hi! I'm line 500 in foo.txt.\n"

Now, on Windows, in a working copy, this line looks like this:

  "Hi! I'm line 500 in foo.txt.\r\n"

Currently, the trailing \n gets eaten, but not the trailing \r. This
means that the blame callback is called with this argument:

  "Hi! I'm line 500 in foo.txt.\r"

When writing this to the 'normal' blame output, this line gets a \n
added, leading to the intended:

  "Hi! I'm line 500 in foo.txt.\r\n"

But that seems wrong. If a client doesn't want to output the newline,
it should be able to do so, resulting in

  "Hi! I'm line 500 in foo.txt."

instead of the current

  "Hi! I'm line 500 in foo.txt.\r"

So, what I'm saying is that it should be possible to tell blame either to send

  "Hi! I'm line 500 in foo.txt."

(no newline, as it does now for \n style eols), or

  "Hi! I'm line 500 in foo.txt.\r\n"

which it currently never does.

What I was proposing is that we create a blame callback which takes an
extra argument 'const char *eol_str' which - in this example - would
point to "\r\n" and be passed the "Hi! I'm line 500 in foo.txt." (ie
without eol marker).

BTW: I just saw there's absolutely *no* output included in the blame
xml, so that's not a problem at all.

I hope this is more clear?

bye,

Erik.

PS: If it's not, never mind, just propose a good way to resolve the
situation and I'll implement it. I myself think it won't be too bad to
choose to include the eol marker in the returned line content. But
that would require revving the svn_client_blame2 api, since the intent
seems to be not to include it.
Received on Wed Dec 7 20:00:09 2005