On Wed, 7 Dec 2005, Erik Huelsmann wrote:
> 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.
You noted below that this is not a rpblem, so I don't need to say that.
Currently, the XML output is irrelevant for this discussion.
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).
I think including the CR in the lines provided to the callback is a bug
and doesn't need to be preserved.
So, we have two choices:
a) Either keep the current bevaviour (but fixed in the CRLF (and CR?)
case)
or
b) Change the behaviour to include the eol markers when calling the
callback. This requires revving the svn_client_bblame2 API for a semantic
change.
a) means that we always normalize the output of svn blame to use the EOL
style of the platform. I think that's fine. If the caller really needs
the actual EOL marker, it can grab the file and use the line number. But
I hope there are more interesting content on the line than the eol
marker:-)
If we choose b), that means that the caller can choose to either output
the exact eol marker or normalize it to the platform's EOL marker. While
this might sound flexible, I have a hard time justifying the new API for
this obscure case. Note that you wouldn't need to provide the eol marker
separately, the caller can easily identify it at the end of the line
itself.
I like solution a). It menas that you don't need to rev the API. Note
that the diff library now supports all eol styles we support. I wonder
how many people use files with CR line endings, but anyway...
Thanks,
//Peter
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Dec 7 22:15:32 2005